* [Qemu-devel] [PATCH v4 0/2] spapr: generate DT node names
@ 2015-09-24 10:27 Laurent Vivier
2015-09-24 10:27 ` [Qemu-devel] [PATCH v4 1/2] PCI: add missing classes in pci_ids.h to build device tree Laurent Vivier
2015-09-24 10:27 ` [Qemu-devel] [PATCH v4 2/2] spapr: generate DT node names Laurent Vivier
0 siblings, 2 replies; 12+ messages in thread
From: Laurent Vivier @ 2015-09-24 10:27 UTC (permalink / raw)
To: qemu-ppc
Cc: Laurent Vivier, thuth, Michael S. Tsirkin, qemu-devel,
Alexander Graf, David Gibson
When DT node names for PCI devices are generated by SLOF,
they are generated according to the type of the device
(for instance, ethernet for virtio-net-pci device).
Node name for hotplugged devices is generated by QEMU.
This series adds the mechanic to QEMU to create the node
name according to the device type too.
v4: move pci_ids.h to a separate patch, fix PCI_CLASS_NETWORK_WORDFIP
remove duplicate NL, remove 386, 486 and alpha subclasses
rename "unknown-legacy-device", correctly check array size
add Thomas and Michael "Reviewed-by".
v3: use values from pci_ids.h, update pci_ids.h values
keep only details for USB (xhci, ohci, ...) and PIC (IO-APIC, IO-XAPIC)
v2: Use CamelCase name, remove misc-* name,
remove _OTHER entries to fallback to class name (as SLOF does).
Fix typo (IPMI-bltr).
Laurent Vivier (2):
PCI: add missing classes in pci_ids.h to build device tree
spapr: generate DT node names
hw/ppc/spapr_pci.c | 292 ++++++++++++++++++++++++++++++++++++++++++++---
include/hw/pci/pci_ids.h | 112 ++++++++++++++++--
2 files changed, 381 insertions(+), 23 deletions(-)
--
2.4.3
^ permalink raw reply [flat|nested] 12+ messages in thread* [Qemu-devel] [PATCH v4 1/2] PCI: add missing classes in pci_ids.h to build device tree 2015-09-24 10:27 [Qemu-devel] [PATCH v4 0/2] spapr: generate DT node names Laurent Vivier @ 2015-09-24 10:27 ` Laurent Vivier 2015-09-24 10:45 ` Thomas Huth 2015-09-24 10:27 ` [Qemu-devel] [PATCH v4 2/2] spapr: generate DT node names Laurent Vivier 1 sibling, 1 reply; 12+ messages in thread From: Laurent Vivier @ 2015-09-24 10:27 UTC (permalink / raw) To: qemu-ppc Cc: Laurent Vivier, thuth, Michael S. Tsirkin, qemu-devel, Alexander Graf, David Gibson To allow QEMU to add PCI entries in device tree, we must have a more exhaustive list of PCI class IDs. This patch synchronizes as much as possible with pci_ids.h and add some missing IDs from SLOF. Signed-off-by: Laurent Vivier <lvivier@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> --- include/hw/pci/pci_ids.h | 112 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 103 insertions(+), 9 deletions(-) diff --git a/include/hw/pci/pci_ids.h b/include/hw/pci/pci_ids.h index d98e6c9..e27dc39 100644 --- a/include/hw/pci/pci_ids.h +++ b/include/hw/pci/pci_ids.h @@ -12,41 +12,84 @@ /* Device classes and subclasses */ -#define PCI_BASE_CLASS_STORAGE 0x01 -#define PCI_BASE_CLASS_NETWORK 0x02 +#define PCI_CLASS_NOT_DEFINED 0x0000 +#define PCI_CLASS_NOT_DEFINED_VGA 0x0001 +#define PCI_BASE_CLASS_STORAGE 0x01 #define PCI_CLASS_STORAGE_SCSI 0x0100 #define PCI_CLASS_STORAGE_IDE 0x0101 +#define PCI_CLASS_STORAGE_FLOPPY 0x0102 +#define PCI_CLASS_STORAGE_IPI 0x0103 #define PCI_CLASS_STORAGE_RAID 0x0104 +#define PCI_CLASS_STORAGE_ATA 0x0105 #define PCI_CLASS_STORAGE_SATA 0x0106 +#define PCI_CLASS_STORAGE_SAS 0x0107 #define PCI_CLASS_STORAGE_EXPRESS 0x0108 #define PCI_CLASS_STORAGE_OTHER 0x0180 +#define PCI_BASE_CLASS_NETWORK 0x02 #define PCI_CLASS_NETWORK_ETHERNET 0x0200 +#define PCI_CLASS_NETWORK_TOKEN_RING 0x0201 +#define PCI_CLASS_NETWORK_FDDI 0x0202 +#define PCI_CLASS_NETWORK_ATM 0x0203 +#define PCI_CLASS_NETWORK_ISDN 0x0204 +#define PCI_CLASS_NETWORK_WORLDFIP 0x0205 +#define PCI_CLASS_NETWORK_PICMG214 0x0206 #define PCI_CLASS_NETWORK_OTHER 0x0280 +#define PCI_BASE_CLASS_DISPLAY 0x03 #define PCI_CLASS_DISPLAY_VGA 0x0300 +#define PCI_CLASS_DISPLAY_XGA 0x0301 +#define PCI_CLASS_DISPLAY_3D 0x0302 #define PCI_CLASS_DISPLAY_OTHER 0x0380 +#define PCI_BASE_CLASS_MULTIMEDIA 0x04 +#define PCI_CLASS_MULTIMEDIA_VIDEO 0x0400 #define PCI_CLASS_MULTIMEDIA_AUDIO 0x0401 +#define PCI_CLASS_MULTIMEDIA_PHONE 0x0402 +#define PCI_CLASS_MULTIMEDIA_OTHER 0x0480 +#define PCI_BASE_CLASS_MEMORY 0x05 #define PCI_CLASS_MEMORY_RAM 0x0500 +#define PCI_CLASS_MEMORY_FLASH 0x0501 +#define PCI_CLASS_MEMORY_OTHER 0x0580 -#define PCI_CLASS_SYSTEM_SDHCI 0x0805 -#define PCI_CLASS_SYSTEM_OTHER 0x0880 - -#define PCI_CLASS_SERIAL_USB 0x0c03 -#define PCI_CLASS_SERIAL_SMBUS 0x0c05 - +#define PCI_BASE_CLASS_BRIDGE 0x06 #define PCI_CLASS_BRIDGE_HOST 0x0600 #define PCI_CLASS_BRIDGE_ISA 0x0601 +#define PCI_CLASS_BRIDGE_EISA 0x0602 +#define PCI_CLASS_BRIDGE_MC 0x0603 #define PCI_CLASS_BRIDGE_PCI 0x0604 #define PCI_CLASS_BRIDGE_PCI_INF_SUB 0x01 +#define PCI_CLASS_BRIDGE_PCMCIA 0x0605 +#define PCI_CLASS_BRIDGE_NUBUS 0x0606 +#define PCI_CLASS_BRIDGE_CARDBUS 0x0607 +#define PCI_CLASS_BRIDGE_RACEWAY 0x0608 +#define PCI_CLASS_BRIDGE_PCI_SEMITP 0x0609 +#define PCI_CLASS_BRIDGE_IB_PCI 0x060a #define PCI_CLASS_BRIDGE_OTHER 0x0680 +#define PCI_BASE_CLASS_COMMUNICATION 0x07 #define PCI_CLASS_COMMUNICATION_SERIAL 0x0700 +#define PCI_CLASS_COMMUNICATION_PARALLEL 0x0701 +#define PCI_CLASS_COMMUNICATION_MULTISERIAL 0x0702 +#define PCI_CLASS_COMMUNICATION_MODEM 0x0703 +#define PCI_CLASS_COMMUNICATION_GPIB 0x0704 +#define PCI_CLASS_COMMUNICATION_SC 0x0705 #define PCI_CLASS_COMMUNICATION_OTHER 0x0780 +#define PCI_BASE_CLASS_SYSTEM 0x08 +#define PCI_CLASS_SYSTEM_PIC 0x0800 +#define PCI_CLASS_SYSTEM_PIC_IOAPIC 0x080010 +#define PCI_CLASS_SYSTEM_PIC_IOXAPIC 0x080020 +#define PCI_CLASS_SYSTEM_DMA 0x0801 +#define PCI_CLASS_SYSTEM_TIMER 0x0802 +#define PCI_CLASS_SYSTEM_RTC 0x0803 +#define PCI_CLASS_SYSTEM_PCI_HOTPLUG 0x0804 +#define PCI_CLASS_SYSTEM_SDHCI 0x0805 +#define PCI_CLASS_SYSTEM_OTHER 0x0880 + +#define PCI_BASE_CLASS_INPUT 0x09 #define PCI_CLASS_INPUT_KEYBOARD 0x0900 #define PCI_CLASS_INPUT_PEN 0x0901 #define PCI_CLASS_INPUT_MOUSE 0x0902 @@ -54,8 +97,59 @@ #define PCI_CLASS_INPUT_GAMEPORT 0x0904 #define PCI_CLASS_INPUT_OTHER 0x0980 -#define PCI_CLASS_PROCESSOR_CO 0x0b40 +#define PCI_BASE_CLASS_DOCKING 0x0a +#define PCI_CLASS_DOCKING_GENERIC 0x0a00 +#define PCI_CLASS_DOCKING_OTHER 0x0a80 + +#define PCI_BASE_CLASS_PROCESSOR 0x0b +#define PCI_CLASS_PROCESSOR_PENTIUM 0x0b02 #define PCI_CLASS_PROCESSOR_POWERPC 0x0b20 +#define PCI_CLASS_PROCESSOR_MIPS 0x0b30 +#define PCI_CLASS_PROCESSOR_CO 0x0b40 + +#define PCI_BASE_CLASS_SERIAL 0x0c +#define PCI_CLASS_SERIAL_FIREWIRE 0x0c00 +#define PCI_CLASS_SERIAL_ACCESS 0x0c01 +#define PCI_CLASS_SERIAL_SSA 0x0c02 +#define PCI_CLASS_SERIAL_USB 0x0c03 +#define PCI_CLASS_SERIAL_USB_UHCI 0x0c0300 +#define PCI_CLASS_SERIAL_USB_OHCI 0x0c0310 +#define PCI_CLASS_SERIAL_USB_EHCI 0x0c0320 +#define PCI_CLASS_SERIAL_USB_XHCI 0x0c0330 +#define PCI_CLASS_SERIAL_USB_UNKNOWN 0x0c0380 +#define PCI_CLASS_SERIAL_USB_DEVICE 0x0c03fe +#define PCI_CLASS_SERIAL_FIBER 0x0c04 +#define PCI_CLASS_SERIAL_SMBUS 0x0c05 +#define PCI_CLASS_SERIAL_IB 0x0c06 +#define PCI_CLASS_SERIAL_IPMI 0x0c07 +#define PCI_CLASS_SERIAL_SERCOS 0x0c08 +#define PCI_CLASS_SERIAL_CANBUS 0x0c09 + +#define PCI_BASE_CLASS_WIRELESS 0x0d +#define PCI_CLASS_WIRELESS_IRDA 0x0d00 +#define PCI_CLASS_WIRELESS_CIR 0x0d01 +#define PCI_CLASS_WIRELESS_RF_CONTROLLER 0x0d10 +#define PCI_CLASS_WIRELESS_BLUETOOTH 0x0d11 +#define PCI_CLASS_WIRELESS_BROADBAND 0x0d12 +#define PCI_CLASS_WIRELESS_OTHER 0x0d80 + +#define PCI_BASE_CLASS_SATELLITE 0x0f +#define PCI_CLASS_SATELLITE_TV 0x0f00 +#define PCI_CLASS_SATELLITE_AUDIO 0x0f01 +#define PCI_CLASS_SATELLITE_VOICE 0x0f03 +#define PCI_CLASS_SATELLITE_DATA 0x0f04 + +#define PCI_BASE_CLASS_CRYPT 0x10 +#define PCI_CLASS_CRYPT_NETWORK 0x1000 +#define PCI_CLASS_CRYPT_ENTERTAINMENT 0x1001 +#define PCI_CLASS_CRYPT_OTHER 0x1080 + +#define PCI_BASE_CLASS_SIGNAL_PROCESSING 0x11 +#define PCI_CLASS_SP_DPIO 0x1100 +#define PCI_CLASS_SP_PERF 0x1101 +#define PCI_CLASS_SP_SYNCH 0x1110 +#define PCI_CLASS_SP_MANAGEMENT 0x1120 +#define PCI_CLASS_SP_OTHER 0x1180 #define PCI_CLASS_OTHERS 0xff -- 2.4.3 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/2] PCI: add missing classes in pci_ids.h to build device tree 2015-09-24 10:27 ` [Qemu-devel] [PATCH v4 1/2] PCI: add missing classes in pci_ids.h to build device tree Laurent Vivier @ 2015-09-24 10:45 ` Thomas Huth 0 siblings, 0 replies; 12+ messages in thread From: Thomas Huth @ 2015-09-24 10:45 UTC (permalink / raw) To: Laurent Vivier, qemu-ppc Cc: qemu-devel, Michael S. Tsirkin, Alexander Graf, David Gibson On 24/09/15 12:27, Laurent Vivier wrote: > To allow QEMU to add PCI entries in device tree, > we must have a more exhaustive list of PCI class IDs. > > This patch synchronizes as much as possible with > pci_ids.h and add some missing IDs from SLOF. > > Signed-off-by: Laurent Vivier <lvivier@redhat.com> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > --- > include/hw/pci/pci_ids.h | 112 +++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 103 insertions(+), 9 deletions(-) Reviewed-by: Thomas Huth <thuth@redhat.com> ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v4 2/2] spapr: generate DT node names 2015-09-24 10:27 [Qemu-devel] [PATCH v4 0/2] spapr: generate DT node names Laurent Vivier 2015-09-24 10:27 ` [Qemu-devel] [PATCH v4 1/2] PCI: add missing classes in pci_ids.h to build device tree Laurent Vivier @ 2015-09-24 10:27 ` Laurent Vivier 2015-09-24 23:29 ` Gavin Shan 2015-09-29 5:18 ` David Gibson 1 sibling, 2 replies; 12+ messages in thread From: Laurent Vivier @ 2015-09-24 10:27 UTC (permalink / raw) To: qemu-ppc Cc: Laurent Vivier, thuth, Michael S. Tsirkin, qemu-devel, Alexander Graf, David Gibson When DT node names for PCI devices are generated by SLOF, they are generated according to the type of the device (for instance, ethernet for virtio-net-pci device). Node name for hotplugged devices is generated by QEMU. This patch adds the mechanic to QEMU to create the node name according to the device type too. The data structure has been roughly copied from OpenBIOS/OpenHackware, node names from SLOF. Example: Hotplugging some PCI cards with QEMU monitor: device_add virtio-tablet-pci device_add virtio-serial-pci device_add virtio-mouse-pci device_add virtio-scsi-pci device_add virtio-gpu-pci device_add ne2k_pci device_add nec-usb-xhci device_add intel-hda What we can see in linux device tree: for dir in /proc/device-tree/pci@800000020000000/*@*/; do echo $dir cat $dir/name echo done WITHOUT this patch: /proc/device-tree/pci@800000020000000/pci@0/ pci /proc/device-tree/pci@800000020000000/pci@1/ pci /proc/device-tree/pci@800000020000000/pci@2/ pci /proc/device-tree/pci@800000020000000/pci@3/ pci /proc/device-tree/pci@800000020000000/pci@4/ pci /proc/device-tree/pci@800000020000000/pci@5/ pci /proc/device-tree/pci@800000020000000/pci@6/ pci /proc/device-tree/pci@800000020000000/pci@7/ pci WITH this patch: /proc/device-tree/pci@800000020000000/communication-controller@1/ communication-controller /proc/device-tree/pci@800000020000000/display@4/ display /proc/device-tree/pci@800000020000000/ethernet@5/ ethernet /proc/device-tree/pci@800000020000000/input-controller@0/ input-controller /proc/device-tree/pci@800000020000000/mouse@2/ mouse /proc/device-tree/pci@800000020000000/multimedia-device@7/ multimedia-device /proc/device-tree/pci@800000020000000/scsi@3/ scsi /proc/device-tree/pci@800000020000000/usb-xhci@6/ usb-xhci Signed-off-by: Laurent Vivier <lvivier@redhat.com> Reviewed-by: Thomas Huth <thuth@redhat.com> --- hw/ppc/spapr_pci.c | 292 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 278 insertions(+), 14 deletions(-) diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index a2feb4c..63eb28c 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -38,6 +38,7 @@ #include "hw/pci/pci_bridge.h" #include "hw/pci/pci_bus.h" +#include "hw/pci/pci_ids.h" #include "hw/ppc/spapr_drc.h" #include "sysemu/device_tree.h" @@ -944,6 +945,276 @@ static void populate_resource_props(PCIDevice *d, ResourceProps *rp) rp->assigned_len = assigned_idx * sizeof(ResourceFields); } +typedef struct PCIClass PCIClass; +typedef struct PCISubClass PCISubClass; +typedef struct PCIIFace PCIIFace; + +struct PCIIFace { + uint8_t iface; + const char *name; +}; + +struct PCISubClass { + uint8_t subclass; + const char *name; + const PCIIFace *iface; +}; +#define SUBCLASS(a) ((uint8_t)a) +#define IFACE(a) ((uint8_t)a) + +struct PCIClass { + const char *name; + const PCISubClass *subc; +}; + +static const PCISubClass undef_subclass[] = { + { IFACE(PCI_CLASS_NOT_DEFINED_VGA), "display", NULL }, + { 0xFF, NULL, NULL, NULL }, +}; + +static const PCISubClass mass_subclass[] = { + { SUBCLASS(PCI_CLASS_STORAGE_SCSI), "scsi", NULL }, + { SUBCLASS(PCI_CLASS_STORAGE_IDE), "ide", NULL }, + { SUBCLASS(PCI_CLASS_STORAGE_FLOPPY), "fdc", NULL }, + { SUBCLASS(PCI_CLASS_STORAGE_IPI), "ipi", NULL }, + { SUBCLASS(PCI_CLASS_STORAGE_RAID), "raid", NULL }, + { SUBCLASS(PCI_CLASS_STORAGE_ATA), "ata", NULL }, + { SUBCLASS(PCI_CLASS_STORAGE_SATA), "sata", NULL }, + { SUBCLASS(PCI_CLASS_STORAGE_SAS), "sas", NULL }, + { 0xFF, NULL, NULL }, +}; + +static const PCISubClass net_subclass[] = { + { SUBCLASS(PCI_CLASS_NETWORK_ETHERNET), "ethernet", NULL }, + { SUBCLASS(PCI_CLASS_NETWORK_TOKEN_RING), "token-ring", NULL }, + { SUBCLASS(PCI_CLASS_NETWORK_FDDI), "fddi", NULL }, + { SUBCLASS(PCI_CLASS_NETWORK_ATM), "atm", NULL }, + { SUBCLASS(PCI_CLASS_NETWORK_ISDN), "isdn", NULL }, + { SUBCLASS(PCI_CLASS_NETWORK_WORLDFIP), "worldfip", NULL }, + { SUBCLASS(PCI_CLASS_NETWORK_PICMG214), "picmg", NULL }, + { 0xFF, NULL, NULL }, +}; + +static const PCISubClass displ_subclass[] = { + { SUBCLASS(PCI_CLASS_DISPLAY_VGA), "vga", NULL }, + { SUBCLASS(PCI_CLASS_DISPLAY_XGA), "xga", NULL }, + { SUBCLASS(PCI_CLASS_DISPLAY_3D), "3d-controller", NULL }, + { 0xFF, NULL, NULL }, +}; + +static const PCISubClass media_subclass[] = { + { SUBCLASS(PCI_CLASS_MULTIMEDIA_VIDEO), "video", NULL }, + { SUBCLASS(PCI_CLASS_MULTIMEDIA_AUDIO), "sound", NULL }, + { SUBCLASS(PCI_CLASS_MULTIMEDIA_PHONE), "telephony", NULL }, + { 0xFF, NULL, NULL }, +}; + +static const PCISubClass mem_subclass[] = { + { SUBCLASS(PCI_CLASS_MEMORY_RAM), "memory", NULL }, + { SUBCLASS(PCI_CLASS_MEMORY_FLASH), "flash", NULL }, + { 0xFF, NULL, NULL }, +}; + +static const PCISubClass bridg_subclass[] = { + { SUBCLASS(PCI_CLASS_BRIDGE_HOST), "host", NULL }, + { SUBCLASS(PCI_CLASS_BRIDGE_ISA), "isa", NULL }, + { SUBCLASS(PCI_CLASS_BRIDGE_EISA), "eisa", NULL }, + { SUBCLASS(PCI_CLASS_BRIDGE_MC), "mca", NULL }, + { SUBCLASS(PCI_CLASS_BRIDGE_PCI), "pci", NULL }, + { SUBCLASS(PCI_CLASS_BRIDGE_PCMCIA), "pcmcia", NULL }, + { SUBCLASS(PCI_CLASS_BRIDGE_NUBUS), "nubus", NULL }, + { SUBCLASS(PCI_CLASS_BRIDGE_CARDBUS), "cardbus", NULL }, + { SUBCLASS(PCI_CLASS_BRIDGE_RACEWAY), "raceway", NULL }, + { SUBCLASS(PCI_CLASS_BRIDGE_PCI_SEMITP), "semi-transparent-pci", NULL }, + { SUBCLASS(PCI_CLASS_BRIDGE_IB_PCI), "infiniband", NULL }, + { 0xFF, NULL, NULL }, +}; + +static const PCISubClass comm_subclass[] = { + { SUBCLASS(PCI_CLASS_COMMUNICATION_SERIAL), "serial", NULL }, + { SUBCLASS(PCI_CLASS_COMMUNICATION_PARALLEL), "parallel", NULL }, + { SUBCLASS(PCI_CLASS_COMMUNICATION_MULTISERIAL), "multiport-serial", NULL }, + { SUBCLASS(PCI_CLASS_COMMUNICATION_MODEM), "modem", NULL }, + { SUBCLASS(PCI_CLASS_COMMUNICATION_GPIB), "gpib", NULL }, + { SUBCLASS(PCI_CLASS_COMMUNICATION_SC), "smart-card", NULL }, + { 0xFF, NULL, NULL, NULL }, +}; + +static const PCIIFace pic_iface[] = { + { IFACE(PCI_CLASS_SYSTEM_PIC_IOAPIC), "io-apic" }, + { IFACE(PCI_CLASS_SYSTEM_PIC_IOXAPIC), "io-xapic" }, + { 0xFF, NULL }, +}; + +static const PCISubClass sys_subclass[] = { + { SUBCLASS(PCI_CLASS_SYSTEM_PIC), "interrupt-controller", pic_iface }, + { SUBCLASS(PCI_CLASS_SYSTEM_DMA), "dma-controller", NULL }, + { SUBCLASS(PCI_CLASS_SYSTEM_TIMER), "timer", NULL }, + { SUBCLASS(PCI_CLASS_SYSTEM_RTC), "rtc", NULL }, + { SUBCLASS(PCI_CLASS_SYSTEM_PCI_HOTPLUG), "hot-plug-controller", NULL }, + { SUBCLASS(PCI_CLASS_SYSTEM_SDHCI), "sd-host-controller", NULL }, + { 0xFF, NULL, NULL }, +}; + +static const PCISubClass inp_subclass[] = { + { SUBCLASS(PCI_CLASS_INPUT_KEYBOARD), "keyboard", NULL }, + { SUBCLASS(PCI_CLASS_INPUT_PEN), "pen", NULL }, + { SUBCLASS(PCI_CLASS_INPUT_MOUSE), "mouse", NULL }, + { SUBCLASS(PCI_CLASS_INPUT_SCANNER), "scanner", NULL }, + { SUBCLASS(PCI_CLASS_INPUT_GAMEPORT), "gameport", NULL }, + { 0xFF, NULL, NULL }, +}; + +static const PCISubClass dock_subclass[] = { + { SUBCLASS(PCI_CLASS_DOCKING_GENERIC), "dock", NULL }, + { 0xFF, NULL, NULL }, +}; + +static const PCISubClass cpu_subclass[] = { + { SUBCLASS(PCI_CLASS_PROCESSOR_PENTIUM), "pentium", NULL }, + { SUBCLASS(PCI_CLASS_PROCESSOR_POWERPC), "powerpc", NULL }, + { SUBCLASS(PCI_CLASS_PROCESSOR_MIPS), "mips", NULL }, + { SUBCLASS(PCI_CLASS_PROCESSOR_CO), "co-processor", NULL }, + { 0xFF, NULL, NULL }, +}; + +static const PCIIFace usb_iface[] = { + { IFACE(PCI_CLASS_SERIAL_USB_UHCI), "usb-uhci" }, + { IFACE(PCI_CLASS_SERIAL_USB_OHCI), "usb-ohci", }, + { IFACE(PCI_CLASS_SERIAL_USB_EHCI), "usb-ehci" }, + { IFACE(PCI_CLASS_SERIAL_USB_XHCI), "usb-xhci" }, + { IFACE(PCI_CLASS_SERIAL_USB_UNKNOWN), "usb-unknown" }, + { IFACE(PCI_CLASS_SERIAL_USB_DEVICE), "usb-device" }, + { 0xFF, NULL }, +}; + +static const PCISubClass ser_subclass[] = { + { SUBCLASS(PCI_CLASS_SERIAL_FIREWIRE), "firewire", NULL }, + { SUBCLASS(PCI_CLASS_SERIAL_ACCESS), "access-bus", NULL }, + { SUBCLASS(PCI_CLASS_SERIAL_SSA), "ssa", NULL }, + { SUBCLASS(PCI_CLASS_SERIAL_USB), "usb", usb_iface }, + { SUBCLASS(PCI_CLASS_SERIAL_FIBER), "fibre-channel", NULL }, + { SUBCLASS(PCI_CLASS_SERIAL_SMBUS), "smb", NULL }, + { SUBCLASS(PCI_CLASS_SERIAL_IB), "infiniband", NULL }, + { SUBCLASS(PCI_CLASS_SERIAL_IPMI), "ipmi", NULL }, + { SUBCLASS(PCI_CLASS_SERIAL_SERCOS), "sercos", NULL }, + { SUBCLASS(PCI_CLASS_SERIAL_CANBUS), "canbus", NULL }, + { 0xFF, NULL, NULL }, +}; + +static const PCISubClass wrl_subclass[] = { + { SUBCLASS(PCI_CLASS_WIRELESS_IRDA), "irda", NULL }, + { SUBCLASS(PCI_CLASS_WIRELESS_CIR), "consumer-ir", NULL }, + { SUBCLASS(PCI_CLASS_WIRELESS_RF_CONTROLLER), "rf-controller", NULL }, + { SUBCLASS(PCI_CLASS_WIRELESS_BLUETOOTH), "bluetooth", NULL }, + { SUBCLASS(PCI_CLASS_WIRELESS_BROADBAND), "broadband", NULL }, + { 0xFF, NULL, NULL }, +}; + +static const PCISubClass sat_subclass[] = { + { SUBCLASS(PCI_CLASS_SATELLITE_TV), "satellite-tv", NULL }, + { SUBCLASS(PCI_CLASS_SATELLITE_AUDIO), "satellite-audio", NULL }, + { SUBCLASS(PCI_CLASS_SATELLITE_VOICE), "satellite-voice", NULL }, + { SUBCLASS(PCI_CLASS_SATELLITE_DATA), "satellite-data", NULL }, + { 0xFF, NULL, NULL }, +}; + +static const PCISubClass crypt_subclass[] = { + { SUBCLASS(PCI_CLASS_CRYPT_NETWORK), "network-encryption", NULL }, + { SUBCLASS(PCI_CLASS_CRYPT_ENTERTAINMENT), + "entertainment-encryption", NULL }, + { 0xFF, NULL, NULL }, +}; + +static const PCISubClass spc_subclass[] = { + { SUBCLASS(PCI_CLASS_SP_DPIO), "dpio", NULL }, + { SUBCLASS(PCI_CLASS_SP_PERF), "counter", NULL }, + { SUBCLASS(PCI_CLASS_SP_SYNCH), "measurement", NULL }, + { SUBCLASS(PCI_CLASS_SP_MANAGEMENT), "management-card", NULL }, + { 0xFF, NULL, NULL }, +}; + +static const PCIClass pci_classes[] = { + { "legacy-device", undef_subclass }, + { "mass-storage", mass_subclass }, + { "network", net_subclass }, + { "display", displ_subclass, }, + { "multimedia-device", media_subclass }, + { "memory-controller", mem_subclass }, + { "unknown-bridge", bridg_subclass }, + { "communication-controller", comm_subclass}, + { "system-peripheral", sys_subclass }, + { "input-controller", inp_subclass }, + { "docking-station", dock_subclass }, + { "cpu", cpu_subclass }, + { "serial-bus", ser_subclass }, + { "wireless-controller", wrl_subclass }, + { "intelligent-io", NULL }, + { "satellite-device", sat_subclass }, + { "encryption", crypt_subclass }, + { "data-processing-controller", spc_subclass }, +}; + +static const char *pci_find_device_name(uint8_t class, uint8_t subclass, + uint8_t iface) +{ + const PCIClass *pclass; + const PCISubClass *psubclass; + const PCIIFace *piface; + const char *name; + + if (class >= ARRAY_SIZE(pci_classes)) { + return "pci"; + } + + pclass = pci_classes + class; + name = pclass->name; + + if (pclass->subc == NULL) { + return name; + } + + psubclass = pclass->subc; + while (psubclass->subclass != 0xff) { + if (psubclass->subclass == subclass) { + name = psubclass->name; + break; + } + psubclass++; + } + + piface = psubclass->iface; + if (piface == NULL) { + return name; + } + while (piface->iface != 0xff) { + if (piface->iface == iface) { + name = piface->name; + break; + } + piface++; + } + + return name; +} + +static void pci_get_node_name(char *nodename, int len, PCIDevice *dev) +{ + int slot = PCI_SLOT(dev->devfn); + int func = PCI_FUNC(dev->devfn); + uint32_t ccode = pci_default_read_config(dev, PCI_CLASS_PROG, 3); + const char *name; + + name = pci_find_device_name((ccode >> 16) & 0xff, (ccode >> 8) & 0xff, + ccode & 0xff); + + if (func != 0) { + snprintf(nodename, len, "%s@%x,%x", name, slot, func); + } else { + snprintf(nodename, len, "%s@%x", name, slot); + } +} + static uint32_t spapr_phb_get_pci_drc_index(sPAPRPHBState *phb, PCIDevice *pdev); @@ -955,6 +1226,7 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset, int pci_status, err; char *buf = NULL; uint32_t drc_index = spapr_phb_get_pci_drc_index(sphb, dev); + uint32_t ccode = pci_default_read_config(dev, PCI_CLASS_PROG, 3); if (pci_default_read_config(dev, PCI_HEADER_TYPE, 1) == PCI_HEADER_TYPE_BRIDGE) { @@ -968,8 +1240,7 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset, pci_default_read_config(dev, PCI_DEVICE_ID, 2))); _FDT(fdt_setprop_cell(fdt, offset, "revision-id", pci_default_read_config(dev, PCI_REVISION_ID, 1))); - _FDT(fdt_setprop_cell(fdt, offset, "class-code", - pci_default_read_config(dev, PCI_CLASS_PROG, 3))); + _FDT(fdt_setprop_cell(fdt, offset, "class-code", ccode)); if (pci_default_read_config(dev, PCI_INTERRUPT_PIN, 1)) { _FDT(fdt_setprop_cell(fdt, offset, "interrupts", pci_default_read_config(dev, PCI_INTERRUPT_PIN, 1))); @@ -1010,11 +1281,10 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset, _FDT(fdt_setprop(fdt, offset, "udf-supported", NULL, 0)); } - /* NOTE: this is normally generated by firmware via path/unit name, - * but in our case we must set it manually since it does not get - * processed by OF beforehand - */ - _FDT(fdt_setprop_string(fdt, offset, "name", "pci")); + _FDT(fdt_setprop_string(fdt, offset, "name", + pci_find_device_name((ccode >> 16) & 0xff, + (ccode >> 8) & 0xff, + ccode & 0xff))); buf = spapr_phb_get_loc_code(sphb, dev); if (!buf) { error_report("Failed setting the ibm,loc-code"); @@ -1051,15 +1321,9 @@ static int spapr_create_pci_child_dt(sPAPRPHBState *phb, PCIDevice *dev, void *fdt, int node_offset) { int offset, ret; - int slot = PCI_SLOT(dev->devfn); - int func = PCI_FUNC(dev->devfn); char nodename[FDT_NAME_MAX]; - if (func != 0) { - snprintf(nodename, FDT_NAME_MAX, "pci@%x,%x", slot, func); - } else { - snprintf(nodename, FDT_NAME_MAX, "pci@%x", slot); - } + pci_get_node_name(nodename, FDT_NAME_MAX, dev); offset = fdt_add_subnode(fdt, node_offset, nodename); ret = spapr_populate_pci_child_dt(dev, fdt, offset, phb); -- 2.4.3 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/2] spapr: generate DT node names 2015-09-24 10:27 ` [Qemu-devel] [PATCH v4 2/2] spapr: generate DT node names Laurent Vivier @ 2015-09-24 23:29 ` Gavin Shan 2015-09-25 8:29 ` Laurent Vivier 2015-09-29 5:18 ` David Gibson 1 sibling, 1 reply; 12+ messages in thread From: Gavin Shan @ 2015-09-24 23:29 UTC (permalink / raw) To: Laurent Vivier Cc: thuth, Michael S. Tsirkin, Alexander Graf, qemu-devel, qemu-ppc, David Gibson On Thu, Sep 24, 2015 at 12:27:39PM +0200, Laurent Vivier wrote: >When DT node names for PCI devices are generated by SLOF, >they are generated according to the type of the device >(for instance, ethernet for virtio-net-pci device). > >Node name for hotplugged devices is generated by QEMU. >This patch adds the mechanic to QEMU to create the node >name according to the device type too. > >The data structure has been roughly copied from OpenBIOS/OpenHackware, >node names from SLOF. > >Example: > >Hotplugging some PCI cards with QEMU monitor: > >device_add virtio-tablet-pci >device_add virtio-serial-pci >device_add virtio-mouse-pci >device_add virtio-scsi-pci >device_add virtio-gpu-pci >device_add ne2k_pci >device_add nec-usb-xhci >device_add intel-hda > >What we can see in linux device tree: > >for dir in /proc/device-tree/pci@800000020000000/*@*/; do > echo $dir > cat $dir/name > echo >done > >WITHOUT this patch: > >/proc/device-tree/pci@800000020000000/pci@0/ >pci >/proc/device-tree/pci@800000020000000/pci@1/ >pci >/proc/device-tree/pci@800000020000000/pci@2/ >pci >/proc/device-tree/pci@800000020000000/pci@3/ >pci >/proc/device-tree/pci@800000020000000/pci@4/ >pci >/proc/device-tree/pci@800000020000000/pci@5/ >pci >/proc/device-tree/pci@800000020000000/pci@6/ >pci >/proc/device-tree/pci@800000020000000/pci@7/ >pci > >WITH this patch: > >/proc/device-tree/pci@800000020000000/communication-controller@1/ >communication-controller >/proc/device-tree/pci@800000020000000/display@4/ >display >/proc/device-tree/pci@800000020000000/ethernet@5/ >ethernet >/proc/device-tree/pci@800000020000000/input-controller@0/ >input-controller >/proc/device-tree/pci@800000020000000/mouse@2/ >mouse >/proc/device-tree/pci@800000020000000/multimedia-device@7/ >multimedia-device >/proc/device-tree/pci@800000020000000/scsi@3/ >scsi >/proc/device-tree/pci@800000020000000/usb-xhci@6/ >usb-xhci > >Signed-off-by: Laurent Vivier <lvivier@redhat.com> >Reviewed-by: Thomas Huth <thuth@redhat.com> >--- > hw/ppc/spapr_pci.c | 292 ++++++++++++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 278 insertions(+), 14 deletions(-) > >diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c >index a2feb4c..63eb28c 100644 >--- a/hw/ppc/spapr_pci.c >+++ b/hw/ppc/spapr_pci.c >@@ -38,6 +38,7 @@ > > #include "hw/pci/pci_bridge.h" > #include "hw/pci/pci_bus.h" >+#include "hw/pci/pci_ids.h" > #include "hw/ppc/spapr_drc.h" > #include "sysemu/device_tree.h" > >@@ -944,6 +945,276 @@ static void populate_resource_props(PCIDevice *d, ResourceProps *rp) > rp->assigned_len = assigned_idx * sizeof(ResourceFields); > } > One question would be: is there one reason why the logic, converting class/subclass/iface code to tring, isn't put into generic PCI module? If the code is put there, all platforms can reuse it. Thanks, Gavin >+typedef struct PCIClass PCIClass; >+typedef struct PCISubClass PCISubClass; >+typedef struct PCIIFace PCIIFace; >+ >+struct PCIIFace { >+ uint8_t iface; >+ const char *name; >+}; >+ >+struct PCISubClass { >+ uint8_t subclass; >+ const char *name; >+ const PCIIFace *iface; >+}; >+#define SUBCLASS(a) ((uint8_t)a) >+#define IFACE(a) ((uint8_t)a) >+ >+struct PCIClass { >+ const char *name; >+ const PCISubClass *subc; >+}; >+ >+static const PCISubClass undef_subclass[] = { >+ { IFACE(PCI_CLASS_NOT_DEFINED_VGA), "display", NULL }, >+ { 0xFF, NULL, NULL, NULL }, >+}; >+ >+static const PCISubClass mass_subclass[] = { >+ { SUBCLASS(PCI_CLASS_STORAGE_SCSI), "scsi", NULL }, >+ { SUBCLASS(PCI_CLASS_STORAGE_IDE), "ide", NULL }, >+ { SUBCLASS(PCI_CLASS_STORAGE_FLOPPY), "fdc", NULL }, >+ { SUBCLASS(PCI_CLASS_STORAGE_IPI), "ipi", NULL }, >+ { SUBCLASS(PCI_CLASS_STORAGE_RAID), "raid", NULL }, >+ { SUBCLASS(PCI_CLASS_STORAGE_ATA), "ata", NULL }, >+ { SUBCLASS(PCI_CLASS_STORAGE_SATA), "sata", NULL }, >+ { SUBCLASS(PCI_CLASS_STORAGE_SAS), "sas", NULL }, >+ { 0xFF, NULL, NULL }, >+}; >+ >+static const PCISubClass net_subclass[] = { >+ { SUBCLASS(PCI_CLASS_NETWORK_ETHERNET), "ethernet", NULL }, >+ { SUBCLASS(PCI_CLASS_NETWORK_TOKEN_RING), "token-ring", NULL }, >+ { SUBCLASS(PCI_CLASS_NETWORK_FDDI), "fddi", NULL }, >+ { SUBCLASS(PCI_CLASS_NETWORK_ATM), "atm", NULL }, >+ { SUBCLASS(PCI_CLASS_NETWORK_ISDN), "isdn", NULL }, >+ { SUBCLASS(PCI_CLASS_NETWORK_WORLDFIP), "worldfip", NULL }, >+ { SUBCLASS(PCI_CLASS_NETWORK_PICMG214), "picmg", NULL }, >+ { 0xFF, NULL, NULL }, >+}; >+ >+static const PCISubClass displ_subclass[] = { >+ { SUBCLASS(PCI_CLASS_DISPLAY_VGA), "vga", NULL }, >+ { SUBCLASS(PCI_CLASS_DISPLAY_XGA), "xga", NULL }, >+ { SUBCLASS(PCI_CLASS_DISPLAY_3D), "3d-controller", NULL }, >+ { 0xFF, NULL, NULL }, >+}; >+ >+static const PCISubClass media_subclass[] = { >+ { SUBCLASS(PCI_CLASS_MULTIMEDIA_VIDEO), "video", NULL }, >+ { SUBCLASS(PCI_CLASS_MULTIMEDIA_AUDIO), "sound", NULL }, >+ { SUBCLASS(PCI_CLASS_MULTIMEDIA_PHONE), "telephony", NULL }, >+ { 0xFF, NULL, NULL }, >+}; >+ >+static const PCISubClass mem_subclass[] = { >+ { SUBCLASS(PCI_CLASS_MEMORY_RAM), "memory", NULL }, >+ { SUBCLASS(PCI_CLASS_MEMORY_FLASH), "flash", NULL }, >+ { 0xFF, NULL, NULL }, >+}; >+ >+static const PCISubClass bridg_subclass[] = { >+ { SUBCLASS(PCI_CLASS_BRIDGE_HOST), "host", NULL }, >+ { SUBCLASS(PCI_CLASS_BRIDGE_ISA), "isa", NULL }, >+ { SUBCLASS(PCI_CLASS_BRIDGE_EISA), "eisa", NULL }, >+ { SUBCLASS(PCI_CLASS_BRIDGE_MC), "mca", NULL }, >+ { SUBCLASS(PCI_CLASS_BRIDGE_PCI), "pci", NULL }, >+ { SUBCLASS(PCI_CLASS_BRIDGE_PCMCIA), "pcmcia", NULL }, >+ { SUBCLASS(PCI_CLASS_BRIDGE_NUBUS), "nubus", NULL }, >+ { SUBCLASS(PCI_CLASS_BRIDGE_CARDBUS), "cardbus", NULL }, >+ { SUBCLASS(PCI_CLASS_BRIDGE_RACEWAY), "raceway", NULL }, >+ { SUBCLASS(PCI_CLASS_BRIDGE_PCI_SEMITP), "semi-transparent-pci", NULL }, >+ { SUBCLASS(PCI_CLASS_BRIDGE_IB_PCI), "infiniband", NULL }, >+ { 0xFF, NULL, NULL }, >+}; >+ >+static const PCISubClass comm_subclass[] = { >+ { SUBCLASS(PCI_CLASS_COMMUNICATION_SERIAL), "serial", NULL }, >+ { SUBCLASS(PCI_CLASS_COMMUNICATION_PARALLEL), "parallel", NULL }, >+ { SUBCLASS(PCI_CLASS_COMMUNICATION_MULTISERIAL), "multiport-serial", NULL }, >+ { SUBCLASS(PCI_CLASS_COMMUNICATION_MODEM), "modem", NULL }, >+ { SUBCLASS(PCI_CLASS_COMMUNICATION_GPIB), "gpib", NULL }, >+ { SUBCLASS(PCI_CLASS_COMMUNICATION_SC), "smart-card", NULL }, >+ { 0xFF, NULL, NULL, NULL }, >+}; >+ >+static const PCIIFace pic_iface[] = { >+ { IFACE(PCI_CLASS_SYSTEM_PIC_IOAPIC), "io-apic" }, >+ { IFACE(PCI_CLASS_SYSTEM_PIC_IOXAPIC), "io-xapic" }, >+ { 0xFF, NULL }, >+}; >+ >+static const PCISubClass sys_subclass[] = { >+ { SUBCLASS(PCI_CLASS_SYSTEM_PIC), "interrupt-controller", pic_iface }, >+ { SUBCLASS(PCI_CLASS_SYSTEM_DMA), "dma-controller", NULL }, >+ { SUBCLASS(PCI_CLASS_SYSTEM_TIMER), "timer", NULL }, >+ { SUBCLASS(PCI_CLASS_SYSTEM_RTC), "rtc", NULL }, >+ { SUBCLASS(PCI_CLASS_SYSTEM_PCI_HOTPLUG), "hot-plug-controller", NULL }, >+ { SUBCLASS(PCI_CLASS_SYSTEM_SDHCI), "sd-host-controller", NULL }, >+ { 0xFF, NULL, NULL }, >+}; >+ >+static const PCISubClass inp_subclass[] = { >+ { SUBCLASS(PCI_CLASS_INPUT_KEYBOARD), "keyboard", NULL }, >+ { SUBCLASS(PCI_CLASS_INPUT_PEN), "pen", NULL }, >+ { SUBCLASS(PCI_CLASS_INPUT_MOUSE), "mouse", NULL }, >+ { SUBCLASS(PCI_CLASS_INPUT_SCANNER), "scanner", NULL }, >+ { SUBCLASS(PCI_CLASS_INPUT_GAMEPORT), "gameport", NULL }, >+ { 0xFF, NULL, NULL }, >+}; >+ >+static const PCISubClass dock_subclass[] = { >+ { SUBCLASS(PCI_CLASS_DOCKING_GENERIC), "dock", NULL }, >+ { 0xFF, NULL, NULL }, >+}; >+ >+static const PCISubClass cpu_subclass[] = { >+ { SUBCLASS(PCI_CLASS_PROCESSOR_PENTIUM), "pentium", NULL }, >+ { SUBCLASS(PCI_CLASS_PROCESSOR_POWERPC), "powerpc", NULL }, >+ { SUBCLASS(PCI_CLASS_PROCESSOR_MIPS), "mips", NULL }, >+ { SUBCLASS(PCI_CLASS_PROCESSOR_CO), "co-processor", NULL }, >+ { 0xFF, NULL, NULL }, >+}; >+ >+static const PCIIFace usb_iface[] = { >+ { IFACE(PCI_CLASS_SERIAL_USB_UHCI), "usb-uhci" }, >+ { IFACE(PCI_CLASS_SERIAL_USB_OHCI), "usb-ohci", }, >+ { IFACE(PCI_CLASS_SERIAL_USB_EHCI), "usb-ehci" }, >+ { IFACE(PCI_CLASS_SERIAL_USB_XHCI), "usb-xhci" }, >+ { IFACE(PCI_CLASS_SERIAL_USB_UNKNOWN), "usb-unknown" }, >+ { IFACE(PCI_CLASS_SERIAL_USB_DEVICE), "usb-device" }, >+ { 0xFF, NULL }, >+}; >+ >+static const PCISubClass ser_subclass[] = { >+ { SUBCLASS(PCI_CLASS_SERIAL_FIREWIRE), "firewire", NULL }, >+ { SUBCLASS(PCI_CLASS_SERIAL_ACCESS), "access-bus", NULL }, >+ { SUBCLASS(PCI_CLASS_SERIAL_SSA), "ssa", NULL }, >+ { SUBCLASS(PCI_CLASS_SERIAL_USB), "usb", usb_iface }, >+ { SUBCLASS(PCI_CLASS_SERIAL_FIBER), "fibre-channel", NULL }, >+ { SUBCLASS(PCI_CLASS_SERIAL_SMBUS), "smb", NULL }, >+ { SUBCLASS(PCI_CLASS_SERIAL_IB), "infiniband", NULL }, >+ { SUBCLASS(PCI_CLASS_SERIAL_IPMI), "ipmi", NULL }, >+ { SUBCLASS(PCI_CLASS_SERIAL_SERCOS), "sercos", NULL }, >+ { SUBCLASS(PCI_CLASS_SERIAL_CANBUS), "canbus", NULL }, >+ { 0xFF, NULL, NULL }, >+}; >+ >+static const PCISubClass wrl_subclass[] = { >+ { SUBCLASS(PCI_CLASS_WIRELESS_IRDA), "irda", NULL }, >+ { SUBCLASS(PCI_CLASS_WIRELESS_CIR), "consumer-ir", NULL }, >+ { SUBCLASS(PCI_CLASS_WIRELESS_RF_CONTROLLER), "rf-controller", NULL }, >+ { SUBCLASS(PCI_CLASS_WIRELESS_BLUETOOTH), "bluetooth", NULL }, >+ { SUBCLASS(PCI_CLASS_WIRELESS_BROADBAND), "broadband", NULL }, >+ { 0xFF, NULL, NULL }, >+}; >+ >+static const PCISubClass sat_subclass[] = { >+ { SUBCLASS(PCI_CLASS_SATELLITE_TV), "satellite-tv", NULL }, >+ { SUBCLASS(PCI_CLASS_SATELLITE_AUDIO), "satellite-audio", NULL }, >+ { SUBCLASS(PCI_CLASS_SATELLITE_VOICE), "satellite-voice", NULL }, >+ { SUBCLASS(PCI_CLASS_SATELLITE_DATA), "satellite-data", NULL }, >+ { 0xFF, NULL, NULL }, >+}; >+ >+static const PCISubClass crypt_subclass[] = { >+ { SUBCLASS(PCI_CLASS_CRYPT_NETWORK), "network-encryption", NULL }, >+ { SUBCLASS(PCI_CLASS_CRYPT_ENTERTAINMENT), >+ "entertainment-encryption", NULL }, >+ { 0xFF, NULL, NULL }, >+}; >+ >+static const PCISubClass spc_subclass[] = { >+ { SUBCLASS(PCI_CLASS_SP_DPIO), "dpio", NULL }, >+ { SUBCLASS(PCI_CLASS_SP_PERF), "counter", NULL }, >+ { SUBCLASS(PCI_CLASS_SP_SYNCH), "measurement", NULL }, >+ { SUBCLASS(PCI_CLASS_SP_MANAGEMENT), "management-card", NULL }, >+ { 0xFF, NULL, NULL }, >+}; >+ >+static const PCIClass pci_classes[] = { >+ { "legacy-device", undef_subclass }, >+ { "mass-storage", mass_subclass }, >+ { "network", net_subclass }, >+ { "display", displ_subclass, }, >+ { "multimedia-device", media_subclass }, >+ { "memory-controller", mem_subclass }, >+ { "unknown-bridge", bridg_subclass }, >+ { "communication-controller", comm_subclass}, >+ { "system-peripheral", sys_subclass }, >+ { "input-controller", inp_subclass }, >+ { "docking-station", dock_subclass }, >+ { "cpu", cpu_subclass }, >+ { "serial-bus", ser_subclass }, >+ { "wireless-controller", wrl_subclass }, >+ { "intelligent-io", NULL }, >+ { "satellite-device", sat_subclass }, >+ { "encryption", crypt_subclass }, >+ { "data-processing-controller", spc_subclass }, >+}; >+ >+static const char *pci_find_device_name(uint8_t class, uint8_t subclass, >+ uint8_t iface) >+{ >+ const PCIClass *pclass; >+ const PCISubClass *psubclass; >+ const PCIIFace *piface; >+ const char *name; >+ >+ if (class >= ARRAY_SIZE(pci_classes)) { >+ return "pci"; >+ } >+ >+ pclass = pci_classes + class; >+ name = pclass->name; >+ >+ if (pclass->subc == NULL) { >+ return name; >+ } >+ >+ psubclass = pclass->subc; >+ while (psubclass->subclass != 0xff) { >+ if (psubclass->subclass == subclass) { >+ name = psubclass->name; >+ break; >+ } >+ psubclass++; >+ } >+ >+ piface = psubclass->iface; >+ if (piface == NULL) { >+ return name; >+ } >+ while (piface->iface != 0xff) { >+ if (piface->iface == iface) { >+ name = piface->name; >+ break; >+ } >+ piface++; >+ } >+ >+ return name; >+} >+ >+static void pci_get_node_name(char *nodename, int len, PCIDevice *dev) >+{ >+ int slot = PCI_SLOT(dev->devfn); >+ int func = PCI_FUNC(dev->devfn); >+ uint32_t ccode = pci_default_read_config(dev, PCI_CLASS_PROG, 3); >+ const char *name; >+ >+ name = pci_find_device_name((ccode >> 16) & 0xff, (ccode >> 8) & 0xff, >+ ccode & 0xff); >+ >+ if (func != 0) { >+ snprintf(nodename, len, "%s@%x,%x", name, slot, func); >+ } else { >+ snprintf(nodename, len, "%s@%x", name, slot); >+ } >+} >+ > static uint32_t spapr_phb_get_pci_drc_index(sPAPRPHBState *phb, > PCIDevice *pdev); > >@@ -955,6 +1226,7 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset, > int pci_status, err; > char *buf = NULL; > uint32_t drc_index = spapr_phb_get_pci_drc_index(sphb, dev); >+ uint32_t ccode = pci_default_read_config(dev, PCI_CLASS_PROG, 3); > > if (pci_default_read_config(dev, PCI_HEADER_TYPE, 1) == > PCI_HEADER_TYPE_BRIDGE) { >@@ -968,8 +1240,7 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset, > pci_default_read_config(dev, PCI_DEVICE_ID, 2))); > _FDT(fdt_setprop_cell(fdt, offset, "revision-id", > pci_default_read_config(dev, PCI_REVISION_ID, 1))); >- _FDT(fdt_setprop_cell(fdt, offset, "class-code", >- pci_default_read_config(dev, PCI_CLASS_PROG, 3))); >+ _FDT(fdt_setprop_cell(fdt, offset, "class-code", ccode)); > if (pci_default_read_config(dev, PCI_INTERRUPT_PIN, 1)) { > _FDT(fdt_setprop_cell(fdt, offset, "interrupts", > pci_default_read_config(dev, PCI_INTERRUPT_PIN, 1))); >@@ -1010,11 +1281,10 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset, > _FDT(fdt_setprop(fdt, offset, "udf-supported", NULL, 0)); > } > >- /* NOTE: this is normally generated by firmware via path/unit name, >- * but in our case we must set it manually since it does not get >- * processed by OF beforehand >- */ >- _FDT(fdt_setprop_string(fdt, offset, "name", "pci")); >+ _FDT(fdt_setprop_string(fdt, offset, "name", >+ pci_find_device_name((ccode >> 16) & 0xff, >+ (ccode >> 8) & 0xff, >+ ccode & 0xff))); > buf = spapr_phb_get_loc_code(sphb, dev); > if (!buf) { > error_report("Failed setting the ibm,loc-code"); >@@ -1051,15 +1321,9 @@ static int spapr_create_pci_child_dt(sPAPRPHBState *phb, PCIDevice *dev, > void *fdt, int node_offset) > { > int offset, ret; >- int slot = PCI_SLOT(dev->devfn); >- int func = PCI_FUNC(dev->devfn); > char nodename[FDT_NAME_MAX]; > >- if (func != 0) { >- snprintf(nodename, FDT_NAME_MAX, "pci@%x,%x", slot, func); >- } else { >- snprintf(nodename, FDT_NAME_MAX, "pci@%x", slot); >- } >+ pci_get_node_name(nodename, FDT_NAME_MAX, dev); > offset = fdt_add_subnode(fdt, node_offset, nodename); > ret = spapr_populate_pci_child_dt(dev, fdt, offset, phb); > >-- >2.4.3 > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/2] spapr: generate DT node names 2015-09-24 23:29 ` Gavin Shan @ 2015-09-25 8:29 ` Laurent Vivier 2015-09-25 9:12 ` Michael S. Tsirkin 0 siblings, 1 reply; 12+ messages in thread From: Laurent Vivier @ 2015-09-25 8:29 UTC (permalink / raw) To: Gavin Shan Cc: thuth, Michael S. Tsirkin, Alexander Graf, qemu-devel, qemu-ppc, David Gibson On 25/09/2015 01:29, Gavin Shan wrote: > On Thu, Sep 24, 2015 at 12:27:39PM +0200, Laurent Vivier wrote: >> When DT node names for PCI devices are generated by SLOF, >> they are generated according to the type of the device >> (for instance, ethernet for virtio-net-pci device). >> >> Node name for hotplugged devices is generated by QEMU. >> This patch adds the mechanic to QEMU to create the node >> name according to the device type too. >> >> The data structure has been roughly copied from OpenBIOS/OpenHackware, >> node names from SLOF. >> >> Example: >> >> Hotplugging some PCI cards with QEMU monitor: >> >> device_add virtio-tablet-pci >> device_add virtio-serial-pci >> device_add virtio-mouse-pci >> device_add virtio-scsi-pci >> device_add virtio-gpu-pci >> device_add ne2k_pci >> device_add nec-usb-xhci >> device_add intel-hda >> >> What we can see in linux device tree: >> >> for dir in /proc/device-tree/pci@800000020000000/*@*/; do >> echo $dir >> cat $dir/name >> echo >> done >> >> WITHOUT this patch: >> >> /proc/device-tree/pci@800000020000000/pci@0/ >> pci >> /proc/device-tree/pci@800000020000000/pci@1/ >> pci >> /proc/device-tree/pci@800000020000000/pci@2/ >> pci >> /proc/device-tree/pci@800000020000000/pci@3/ >> pci >> /proc/device-tree/pci@800000020000000/pci@4/ >> pci >> /proc/device-tree/pci@800000020000000/pci@5/ >> pci >> /proc/device-tree/pci@800000020000000/pci@6/ >> pci >> /proc/device-tree/pci@800000020000000/pci@7/ >> pci >> >> WITH this patch: >> >> /proc/device-tree/pci@800000020000000/communication-controller@1/ >> communication-controller >> /proc/device-tree/pci@800000020000000/display@4/ >> display >> /proc/device-tree/pci@800000020000000/ethernet@5/ >> ethernet >> /proc/device-tree/pci@800000020000000/input-controller@0/ >> input-controller >> /proc/device-tree/pci@800000020000000/mouse@2/ >> mouse >> /proc/device-tree/pci@800000020000000/multimedia-device@7/ >> multimedia-device >> /proc/device-tree/pci@800000020000000/scsi@3/ >> scsi >> /proc/device-tree/pci@800000020000000/usb-xhci@6/ >> usb-xhci >> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com> >> Reviewed-by: Thomas Huth <thuth@redhat.com> >> --- >> hw/ppc/spapr_pci.c | 292 ++++++++++++++++++++++++++++++++++++++++++++++++++--- >> 1 file changed, 278 insertions(+), 14 deletions(-) >> >> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c >> index a2feb4c..63eb28c 100644 >> --- a/hw/ppc/spapr_pci.c >> +++ b/hw/ppc/spapr_pci.c >> @@ -38,6 +38,7 @@ >> >> #include "hw/pci/pci_bridge.h" >> #include "hw/pci/pci_bus.h" >> +#include "hw/pci/pci_ids.h" >> #include "hw/ppc/spapr_drc.h" >> #include "sysemu/device_tree.h" >> >> @@ -944,6 +945,276 @@ static void populate_resource_props(PCIDevice *d, ResourceProps *rp) >> rp->assigned_len = assigned_idx * sizeof(ResourceFields); >> } >> > > One question would be: is there one reason why the logic, converting > class/subclass/iface code to tring, isn't put into generic PCI module? > If the code is put there, all platforms can reuse it. For the moment, it is only used by the device tree generation for spapr, and moreover the names are ones from the openfirmware specification, so except if openbios on sparc/macintosh takes its device tree from QEMU I see no reason to put this in a generic PCI module. Laurent > > Thanks, > Gavin > >> +typedef struct PCIClass PCIClass; >> +typedef struct PCISubClass PCISubClass; >> +typedef struct PCIIFace PCIIFace; >> + >> +struct PCIIFace { >> + uint8_t iface; >> + const char *name; >> +}; >> + >> +struct PCISubClass { >> + uint8_t subclass; >> + const char *name; >> + const PCIIFace *iface; >> +}; >> +#define SUBCLASS(a) ((uint8_t)a) >> +#define IFACE(a) ((uint8_t)a) >> + >> +struct PCIClass { >> + const char *name; >> + const PCISubClass *subc; >> +}; >> + >> +static const PCISubClass undef_subclass[] = { >> + { IFACE(PCI_CLASS_NOT_DEFINED_VGA), "display", NULL }, >> + { 0xFF, NULL, NULL, NULL }, >> +}; >> + >> +static const PCISubClass mass_subclass[] = { >> + { SUBCLASS(PCI_CLASS_STORAGE_SCSI), "scsi", NULL }, >> + { SUBCLASS(PCI_CLASS_STORAGE_IDE), "ide", NULL }, >> + { SUBCLASS(PCI_CLASS_STORAGE_FLOPPY), "fdc", NULL }, >> + { SUBCLASS(PCI_CLASS_STORAGE_IPI), "ipi", NULL }, >> + { SUBCLASS(PCI_CLASS_STORAGE_RAID), "raid", NULL }, >> + { SUBCLASS(PCI_CLASS_STORAGE_ATA), "ata", NULL }, >> + { SUBCLASS(PCI_CLASS_STORAGE_SATA), "sata", NULL }, >> + { SUBCLASS(PCI_CLASS_STORAGE_SAS), "sas", NULL }, >> + { 0xFF, NULL, NULL }, >> +}; >> + >> +static const PCISubClass net_subclass[] = { >> + { SUBCLASS(PCI_CLASS_NETWORK_ETHERNET), "ethernet", NULL }, >> + { SUBCLASS(PCI_CLASS_NETWORK_TOKEN_RING), "token-ring", NULL }, >> + { SUBCLASS(PCI_CLASS_NETWORK_FDDI), "fddi", NULL }, >> + { SUBCLASS(PCI_CLASS_NETWORK_ATM), "atm", NULL }, >> + { SUBCLASS(PCI_CLASS_NETWORK_ISDN), "isdn", NULL }, >> + { SUBCLASS(PCI_CLASS_NETWORK_WORLDFIP), "worldfip", NULL }, >> + { SUBCLASS(PCI_CLASS_NETWORK_PICMG214), "picmg", NULL }, >> + { 0xFF, NULL, NULL }, >> +}; >> + >> +static const PCISubClass displ_subclass[] = { >> + { SUBCLASS(PCI_CLASS_DISPLAY_VGA), "vga", NULL }, >> + { SUBCLASS(PCI_CLASS_DISPLAY_XGA), "xga", NULL }, >> + { SUBCLASS(PCI_CLASS_DISPLAY_3D), "3d-controller", NULL }, >> + { 0xFF, NULL, NULL }, >> +}; >> + >> +static const PCISubClass media_subclass[] = { >> + { SUBCLASS(PCI_CLASS_MULTIMEDIA_VIDEO), "video", NULL }, >> + { SUBCLASS(PCI_CLASS_MULTIMEDIA_AUDIO), "sound", NULL }, >> + { SUBCLASS(PCI_CLASS_MULTIMEDIA_PHONE), "telephony", NULL }, >> + { 0xFF, NULL, NULL }, >> +}; >> + >> +static const PCISubClass mem_subclass[] = { >> + { SUBCLASS(PCI_CLASS_MEMORY_RAM), "memory", NULL }, >> + { SUBCLASS(PCI_CLASS_MEMORY_FLASH), "flash", NULL }, >> + { 0xFF, NULL, NULL }, >> +}; >> + >> +static const PCISubClass bridg_subclass[] = { >> + { SUBCLASS(PCI_CLASS_BRIDGE_HOST), "host", NULL }, >> + { SUBCLASS(PCI_CLASS_BRIDGE_ISA), "isa", NULL }, >> + { SUBCLASS(PCI_CLASS_BRIDGE_EISA), "eisa", NULL }, >> + { SUBCLASS(PCI_CLASS_BRIDGE_MC), "mca", NULL }, >> + { SUBCLASS(PCI_CLASS_BRIDGE_PCI), "pci", NULL }, >> + { SUBCLASS(PCI_CLASS_BRIDGE_PCMCIA), "pcmcia", NULL }, >> + { SUBCLASS(PCI_CLASS_BRIDGE_NUBUS), "nubus", NULL }, >> + { SUBCLASS(PCI_CLASS_BRIDGE_CARDBUS), "cardbus", NULL }, >> + { SUBCLASS(PCI_CLASS_BRIDGE_RACEWAY), "raceway", NULL }, >> + { SUBCLASS(PCI_CLASS_BRIDGE_PCI_SEMITP), "semi-transparent-pci", NULL }, >> + { SUBCLASS(PCI_CLASS_BRIDGE_IB_PCI), "infiniband", NULL }, >> + { 0xFF, NULL, NULL }, >> +}; >> + >> +static const PCISubClass comm_subclass[] = { >> + { SUBCLASS(PCI_CLASS_COMMUNICATION_SERIAL), "serial", NULL }, >> + { SUBCLASS(PCI_CLASS_COMMUNICATION_PARALLEL), "parallel", NULL }, >> + { SUBCLASS(PCI_CLASS_COMMUNICATION_MULTISERIAL), "multiport-serial", NULL }, >> + { SUBCLASS(PCI_CLASS_COMMUNICATION_MODEM), "modem", NULL }, >> + { SUBCLASS(PCI_CLASS_COMMUNICATION_GPIB), "gpib", NULL }, >> + { SUBCLASS(PCI_CLASS_COMMUNICATION_SC), "smart-card", NULL }, >> + { 0xFF, NULL, NULL, NULL }, >> +}; >> + >> +static const PCIIFace pic_iface[] = { >> + { IFACE(PCI_CLASS_SYSTEM_PIC_IOAPIC), "io-apic" }, >> + { IFACE(PCI_CLASS_SYSTEM_PIC_IOXAPIC), "io-xapic" }, >> + { 0xFF, NULL }, >> +}; >> + >> +static const PCISubClass sys_subclass[] = { >> + { SUBCLASS(PCI_CLASS_SYSTEM_PIC), "interrupt-controller", pic_iface }, >> + { SUBCLASS(PCI_CLASS_SYSTEM_DMA), "dma-controller", NULL }, >> + { SUBCLASS(PCI_CLASS_SYSTEM_TIMER), "timer", NULL }, >> + { SUBCLASS(PCI_CLASS_SYSTEM_RTC), "rtc", NULL }, >> + { SUBCLASS(PCI_CLASS_SYSTEM_PCI_HOTPLUG), "hot-plug-controller", NULL }, >> + { SUBCLASS(PCI_CLASS_SYSTEM_SDHCI), "sd-host-controller", NULL }, >> + { 0xFF, NULL, NULL }, >> +}; >> + >> +static const PCISubClass inp_subclass[] = { >> + { SUBCLASS(PCI_CLASS_INPUT_KEYBOARD), "keyboard", NULL }, >> + { SUBCLASS(PCI_CLASS_INPUT_PEN), "pen", NULL }, >> + { SUBCLASS(PCI_CLASS_INPUT_MOUSE), "mouse", NULL }, >> + { SUBCLASS(PCI_CLASS_INPUT_SCANNER), "scanner", NULL }, >> + { SUBCLASS(PCI_CLASS_INPUT_GAMEPORT), "gameport", NULL }, >> + { 0xFF, NULL, NULL }, >> +}; >> + >> +static const PCISubClass dock_subclass[] = { >> + { SUBCLASS(PCI_CLASS_DOCKING_GENERIC), "dock", NULL }, >> + { 0xFF, NULL, NULL }, >> +}; >> + >> +static const PCISubClass cpu_subclass[] = { >> + { SUBCLASS(PCI_CLASS_PROCESSOR_PENTIUM), "pentium", NULL }, >> + { SUBCLASS(PCI_CLASS_PROCESSOR_POWERPC), "powerpc", NULL }, >> + { SUBCLASS(PCI_CLASS_PROCESSOR_MIPS), "mips", NULL }, >> + { SUBCLASS(PCI_CLASS_PROCESSOR_CO), "co-processor", NULL }, >> + { 0xFF, NULL, NULL }, >> +}; >> + >> +static const PCIIFace usb_iface[] = { >> + { IFACE(PCI_CLASS_SERIAL_USB_UHCI), "usb-uhci" }, >> + { IFACE(PCI_CLASS_SERIAL_USB_OHCI), "usb-ohci", }, >> + { IFACE(PCI_CLASS_SERIAL_USB_EHCI), "usb-ehci" }, >> + { IFACE(PCI_CLASS_SERIAL_USB_XHCI), "usb-xhci" }, >> + { IFACE(PCI_CLASS_SERIAL_USB_UNKNOWN), "usb-unknown" }, >> + { IFACE(PCI_CLASS_SERIAL_USB_DEVICE), "usb-device" }, >> + { 0xFF, NULL }, >> +}; >> + >> +static const PCISubClass ser_subclass[] = { >> + { SUBCLASS(PCI_CLASS_SERIAL_FIREWIRE), "firewire", NULL }, >> + { SUBCLASS(PCI_CLASS_SERIAL_ACCESS), "access-bus", NULL }, >> + { SUBCLASS(PCI_CLASS_SERIAL_SSA), "ssa", NULL }, >> + { SUBCLASS(PCI_CLASS_SERIAL_USB), "usb", usb_iface }, >> + { SUBCLASS(PCI_CLASS_SERIAL_FIBER), "fibre-channel", NULL }, >> + { SUBCLASS(PCI_CLASS_SERIAL_SMBUS), "smb", NULL }, >> + { SUBCLASS(PCI_CLASS_SERIAL_IB), "infiniband", NULL }, >> + { SUBCLASS(PCI_CLASS_SERIAL_IPMI), "ipmi", NULL }, >> + { SUBCLASS(PCI_CLASS_SERIAL_SERCOS), "sercos", NULL }, >> + { SUBCLASS(PCI_CLASS_SERIAL_CANBUS), "canbus", NULL }, >> + { 0xFF, NULL, NULL }, >> +}; >> + >> +static const PCISubClass wrl_subclass[] = { >> + { SUBCLASS(PCI_CLASS_WIRELESS_IRDA), "irda", NULL }, >> + { SUBCLASS(PCI_CLASS_WIRELESS_CIR), "consumer-ir", NULL }, >> + { SUBCLASS(PCI_CLASS_WIRELESS_RF_CONTROLLER), "rf-controller", NULL }, >> + { SUBCLASS(PCI_CLASS_WIRELESS_BLUETOOTH), "bluetooth", NULL }, >> + { SUBCLASS(PCI_CLASS_WIRELESS_BROADBAND), "broadband", NULL }, >> + { 0xFF, NULL, NULL }, >> +}; >> + >> +static const PCISubClass sat_subclass[] = { >> + { SUBCLASS(PCI_CLASS_SATELLITE_TV), "satellite-tv", NULL }, >> + { SUBCLASS(PCI_CLASS_SATELLITE_AUDIO), "satellite-audio", NULL }, >> + { SUBCLASS(PCI_CLASS_SATELLITE_VOICE), "satellite-voice", NULL }, >> + { SUBCLASS(PCI_CLASS_SATELLITE_DATA), "satellite-data", NULL }, >> + { 0xFF, NULL, NULL }, >> +}; >> + >> +static const PCISubClass crypt_subclass[] = { >> + { SUBCLASS(PCI_CLASS_CRYPT_NETWORK), "network-encryption", NULL }, >> + { SUBCLASS(PCI_CLASS_CRYPT_ENTERTAINMENT), >> + "entertainment-encryption", NULL }, >> + { 0xFF, NULL, NULL }, >> +}; >> + >> +static const PCISubClass spc_subclass[] = { >> + { SUBCLASS(PCI_CLASS_SP_DPIO), "dpio", NULL }, >> + { SUBCLASS(PCI_CLASS_SP_PERF), "counter", NULL }, >> + { SUBCLASS(PCI_CLASS_SP_SYNCH), "measurement", NULL }, >> + { SUBCLASS(PCI_CLASS_SP_MANAGEMENT), "management-card", NULL }, >> + { 0xFF, NULL, NULL }, >> +}; >> + >> +static const PCIClass pci_classes[] = { >> + { "legacy-device", undef_subclass }, >> + { "mass-storage", mass_subclass }, >> + { "network", net_subclass }, >> + { "display", displ_subclass, }, >> + { "multimedia-device", media_subclass }, >> + { "memory-controller", mem_subclass }, >> + { "unknown-bridge", bridg_subclass }, >> + { "communication-controller", comm_subclass}, >> + { "system-peripheral", sys_subclass }, >> + { "input-controller", inp_subclass }, >> + { "docking-station", dock_subclass }, >> + { "cpu", cpu_subclass }, >> + { "serial-bus", ser_subclass }, >> + { "wireless-controller", wrl_subclass }, >> + { "intelligent-io", NULL }, >> + { "satellite-device", sat_subclass }, >> + { "encryption", crypt_subclass }, >> + { "data-processing-controller", spc_subclass }, >> +}; >> + >> +static const char *pci_find_device_name(uint8_t class, uint8_t subclass, >> + uint8_t iface) >> +{ >> + const PCIClass *pclass; >> + const PCISubClass *psubclass; >> + const PCIIFace *piface; >> + const char *name; >> + >> + if (class >= ARRAY_SIZE(pci_classes)) { >> + return "pci"; >> + } >> + >> + pclass = pci_classes + class; >> + name = pclass->name; >> + >> + if (pclass->subc == NULL) { >> + return name; >> + } >> + >> + psubclass = pclass->subc; >> + while (psubclass->subclass != 0xff) { >> + if (psubclass->subclass == subclass) { >> + name = psubclass->name; >> + break; >> + } >> + psubclass++; >> + } >> + >> + piface = psubclass->iface; >> + if (piface == NULL) { >> + return name; >> + } >> + while (piface->iface != 0xff) { >> + if (piface->iface == iface) { >> + name = piface->name; >> + break; >> + } >> + piface++; >> + } >> + >> + return name; >> +} >> + >> +static void pci_get_node_name(char *nodename, int len, PCIDevice *dev) >> +{ >> + int slot = PCI_SLOT(dev->devfn); >> + int func = PCI_FUNC(dev->devfn); >> + uint32_t ccode = pci_default_read_config(dev, PCI_CLASS_PROG, 3); >> + const char *name; >> + >> + name = pci_find_device_name((ccode >> 16) & 0xff, (ccode >> 8) & 0xff, >> + ccode & 0xff); >> + >> + if (func != 0) { >> + snprintf(nodename, len, "%s@%x,%x", name, slot, func); >> + } else { >> + snprintf(nodename, len, "%s@%x", name, slot); >> + } >> +} >> + >> static uint32_t spapr_phb_get_pci_drc_index(sPAPRPHBState *phb, >> PCIDevice *pdev); >> >> @@ -955,6 +1226,7 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset, >> int pci_status, err; >> char *buf = NULL; >> uint32_t drc_index = spapr_phb_get_pci_drc_index(sphb, dev); >> + uint32_t ccode = pci_default_read_config(dev, PCI_CLASS_PROG, 3); >> >> if (pci_default_read_config(dev, PCI_HEADER_TYPE, 1) == >> PCI_HEADER_TYPE_BRIDGE) { >> @@ -968,8 +1240,7 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset, >> pci_default_read_config(dev, PCI_DEVICE_ID, 2))); >> _FDT(fdt_setprop_cell(fdt, offset, "revision-id", >> pci_default_read_config(dev, PCI_REVISION_ID, 1))); >> - _FDT(fdt_setprop_cell(fdt, offset, "class-code", >> - pci_default_read_config(dev, PCI_CLASS_PROG, 3))); >> + _FDT(fdt_setprop_cell(fdt, offset, "class-code", ccode)); >> if (pci_default_read_config(dev, PCI_INTERRUPT_PIN, 1)) { >> _FDT(fdt_setprop_cell(fdt, offset, "interrupts", >> pci_default_read_config(dev, PCI_INTERRUPT_PIN, 1))); >> @@ -1010,11 +1281,10 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset, >> _FDT(fdt_setprop(fdt, offset, "udf-supported", NULL, 0)); >> } >> >> - /* NOTE: this is normally generated by firmware via path/unit name, >> - * but in our case we must set it manually since it does not get >> - * processed by OF beforehand >> - */ >> - _FDT(fdt_setprop_string(fdt, offset, "name", "pci")); >> + _FDT(fdt_setprop_string(fdt, offset, "name", >> + pci_find_device_name((ccode >> 16) & 0xff, >> + (ccode >> 8) & 0xff, >> + ccode & 0xff))); >> buf = spapr_phb_get_loc_code(sphb, dev); >> if (!buf) { >> error_report("Failed setting the ibm,loc-code"); >> @@ -1051,15 +1321,9 @@ static int spapr_create_pci_child_dt(sPAPRPHBState *phb, PCIDevice *dev, >> void *fdt, int node_offset) >> { >> int offset, ret; >> - int slot = PCI_SLOT(dev->devfn); >> - int func = PCI_FUNC(dev->devfn); >> char nodename[FDT_NAME_MAX]; >> >> - if (func != 0) { >> - snprintf(nodename, FDT_NAME_MAX, "pci@%x,%x", slot, func); >> - } else { >> - snprintf(nodename, FDT_NAME_MAX, "pci@%x", slot); >> - } >> + pci_get_node_name(nodename, FDT_NAME_MAX, dev); >> offset = fdt_add_subnode(fdt, node_offset, nodename); >> ret = spapr_populate_pci_child_dt(dev, fdt, offset, phb); >> >> -- >> 2.4.3 >> >> > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/2] spapr: generate DT node names 2015-09-25 8:29 ` Laurent Vivier @ 2015-09-25 9:12 ` Michael S. Tsirkin 2015-09-25 10:21 ` Laurent Vivier 0 siblings, 1 reply; 12+ messages in thread From: Michael S. Tsirkin @ 2015-09-25 9:12 UTC (permalink / raw) To: Laurent Vivier Cc: thuth, qemu-devel, Alexander Graf, Gavin Shan, qemu-ppc, David Gibson On Fri, Sep 25, 2015 at 10:29:41AM +0200, Laurent Vivier wrote: > > > On 25/09/2015 01:29, Gavin Shan wrote: > > On Thu, Sep 24, 2015 at 12:27:39PM +0200, Laurent Vivier wrote: > >> When DT node names for PCI devices are generated by SLOF, > >> they are generated according to the type of the device > >> (for instance, ethernet for virtio-net-pci device). > >> > >> Node name for hotplugged devices is generated by QEMU. > >> This patch adds the mechanic to QEMU to create the node > >> name according to the device type too. > >> > >> The data structure has been roughly copied from OpenBIOS/OpenHackware, > >> node names from SLOF. > >> > >> Example: > >> > >> Hotplugging some PCI cards with QEMU monitor: > >> > >> device_add virtio-tablet-pci > >> device_add virtio-serial-pci > >> device_add virtio-mouse-pci > >> device_add virtio-scsi-pci > >> device_add virtio-gpu-pci > >> device_add ne2k_pci > >> device_add nec-usb-xhci > >> device_add intel-hda > >> > >> What we can see in linux device tree: > >> > >> for dir in /proc/device-tree/pci@800000020000000/*@*/; do > >> echo $dir > >> cat $dir/name > >> echo > >> done > >> > >> WITHOUT this patch: > >> > >> /proc/device-tree/pci@800000020000000/pci@0/ > >> pci > >> /proc/device-tree/pci@800000020000000/pci@1/ > >> pci > >> /proc/device-tree/pci@800000020000000/pci@2/ > >> pci > >> /proc/device-tree/pci@800000020000000/pci@3/ > >> pci > >> /proc/device-tree/pci@800000020000000/pci@4/ > >> pci > >> /proc/device-tree/pci@800000020000000/pci@5/ > >> pci > >> /proc/device-tree/pci@800000020000000/pci@6/ > >> pci > >> /proc/device-tree/pci@800000020000000/pci@7/ > >> pci > >> > >> WITH this patch: > >> > >> /proc/device-tree/pci@800000020000000/communication-controller@1/ > >> communication-controller > >> /proc/device-tree/pci@800000020000000/display@4/ > >> display > >> /proc/device-tree/pci@800000020000000/ethernet@5/ > >> ethernet > >> /proc/device-tree/pci@800000020000000/input-controller@0/ > >> input-controller > >> /proc/device-tree/pci@800000020000000/mouse@2/ > >> mouse > >> /proc/device-tree/pci@800000020000000/multimedia-device@7/ > >> multimedia-device > >> /proc/device-tree/pci@800000020000000/scsi@3/ > >> scsi > >> /proc/device-tree/pci@800000020000000/usb-xhci@6/ > >> usb-xhci > >> > >> Signed-off-by: Laurent Vivier <lvivier@redhat.com> > >> Reviewed-by: Thomas Huth <thuth@redhat.com> > >> --- > >> hw/ppc/spapr_pci.c | 292 ++++++++++++++++++++++++++++++++++++++++++++++++++--- > >> 1 file changed, 278 insertions(+), 14 deletions(-) > >> > >> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > >> index a2feb4c..63eb28c 100644 > >> --- a/hw/ppc/spapr_pci.c > >> +++ b/hw/ppc/spapr_pci.c > >> @@ -38,6 +38,7 @@ > >> > >> #include "hw/pci/pci_bridge.h" > >> #include "hw/pci/pci_bus.h" > >> +#include "hw/pci/pci_ids.h" > >> #include "hw/ppc/spapr_drc.h" > >> #include "sysemu/device_tree.h" > >> > >> @@ -944,6 +945,276 @@ static void populate_resource_props(PCIDevice *d, ResourceProps *rp) > >> rp->assigned_len = assigned_idx * sizeof(ResourceFields); > >> } > >> > > > > One question would be: is there one reason why the logic, converting > > class/subclass/iface code to tring, isn't put into generic PCI module? > > If the code is put there, all platforms can reuse it. > > For the moment, it is only used by the device tree generation for spapr, > and moreover the names are ones from the openfirmware specification, so > except if openbios on sparc/macintosh takes its device tree from QEMU I > see no reason to put this in a generic PCI module. > > Laurent Our boot paths are supposed to use OF compatible names. I note they look different for some reason. > > > > Thanks, > > Gavin > > > >> +typedef struct PCIClass PCIClass; > >> +typedef struct PCISubClass PCISubClass; > >> +typedef struct PCIIFace PCIIFace; > >> + > >> +struct PCIIFace { > >> + uint8_t iface; > >> + const char *name; > >> +}; > >> + > >> +struct PCISubClass { > >> + uint8_t subclass; > >> + const char *name; > >> + const PCIIFace *iface; > >> +}; > >> +#define SUBCLASS(a) ((uint8_t)a) > >> +#define IFACE(a) ((uint8_t)a) > >> + > >> +struct PCIClass { > >> + const char *name; > >> + const PCISubClass *subc; > >> +}; > >> + > >> +static const PCISubClass undef_subclass[] = { > >> + { IFACE(PCI_CLASS_NOT_DEFINED_VGA), "display", NULL }, > >> + { 0xFF, NULL, NULL, NULL }, > >> +}; > >> + > >> +static const PCISubClass mass_subclass[] = { > >> + { SUBCLASS(PCI_CLASS_STORAGE_SCSI), "scsi", NULL }, > >> + { SUBCLASS(PCI_CLASS_STORAGE_IDE), "ide", NULL }, > >> + { SUBCLASS(PCI_CLASS_STORAGE_FLOPPY), "fdc", NULL }, > >> + { SUBCLASS(PCI_CLASS_STORAGE_IPI), "ipi", NULL }, > >> + { SUBCLASS(PCI_CLASS_STORAGE_RAID), "raid", NULL }, > >> + { SUBCLASS(PCI_CLASS_STORAGE_ATA), "ata", NULL }, > >> + { SUBCLASS(PCI_CLASS_STORAGE_SATA), "sata", NULL }, > >> + { SUBCLASS(PCI_CLASS_STORAGE_SAS), "sas", NULL }, > >> + { 0xFF, NULL, NULL }, > >> +}; > >> + > >> +static const PCISubClass net_subclass[] = { > >> + { SUBCLASS(PCI_CLASS_NETWORK_ETHERNET), "ethernet", NULL }, > >> + { SUBCLASS(PCI_CLASS_NETWORK_TOKEN_RING), "token-ring", NULL }, > >> + { SUBCLASS(PCI_CLASS_NETWORK_FDDI), "fddi", NULL }, > >> + { SUBCLASS(PCI_CLASS_NETWORK_ATM), "atm", NULL }, > >> + { SUBCLASS(PCI_CLASS_NETWORK_ISDN), "isdn", NULL }, > >> + { SUBCLASS(PCI_CLASS_NETWORK_WORLDFIP), "worldfip", NULL }, > >> + { SUBCLASS(PCI_CLASS_NETWORK_PICMG214), "picmg", NULL }, > >> + { 0xFF, NULL, NULL }, > >> +}; > >> + > >> +static const PCISubClass displ_subclass[] = { > >> + { SUBCLASS(PCI_CLASS_DISPLAY_VGA), "vga", NULL }, > >> + { SUBCLASS(PCI_CLASS_DISPLAY_XGA), "xga", NULL }, > >> + { SUBCLASS(PCI_CLASS_DISPLAY_3D), "3d-controller", NULL }, > >> + { 0xFF, NULL, NULL }, > >> +}; > >> + > >> +static const PCISubClass media_subclass[] = { > >> + { SUBCLASS(PCI_CLASS_MULTIMEDIA_VIDEO), "video", NULL }, > >> + { SUBCLASS(PCI_CLASS_MULTIMEDIA_AUDIO), "sound", NULL }, > >> + { SUBCLASS(PCI_CLASS_MULTIMEDIA_PHONE), "telephony", NULL }, > >> + { 0xFF, NULL, NULL }, > >> +}; > >> + > >> +static const PCISubClass mem_subclass[] = { > >> + { SUBCLASS(PCI_CLASS_MEMORY_RAM), "memory", NULL }, > >> + { SUBCLASS(PCI_CLASS_MEMORY_FLASH), "flash", NULL }, > >> + { 0xFF, NULL, NULL }, > >> +}; > >> + > >> +static const PCISubClass bridg_subclass[] = { > >> + { SUBCLASS(PCI_CLASS_BRIDGE_HOST), "host", NULL }, > >> + { SUBCLASS(PCI_CLASS_BRIDGE_ISA), "isa", NULL }, > >> + { SUBCLASS(PCI_CLASS_BRIDGE_EISA), "eisa", NULL }, > >> + { SUBCLASS(PCI_CLASS_BRIDGE_MC), "mca", NULL }, > >> + { SUBCLASS(PCI_CLASS_BRIDGE_PCI), "pci", NULL }, > >> + { SUBCLASS(PCI_CLASS_BRIDGE_PCMCIA), "pcmcia", NULL }, > >> + { SUBCLASS(PCI_CLASS_BRIDGE_NUBUS), "nubus", NULL }, > >> + { SUBCLASS(PCI_CLASS_BRIDGE_CARDBUS), "cardbus", NULL }, > >> + { SUBCLASS(PCI_CLASS_BRIDGE_RACEWAY), "raceway", NULL }, > >> + { SUBCLASS(PCI_CLASS_BRIDGE_PCI_SEMITP), "semi-transparent-pci", NULL }, > >> + { SUBCLASS(PCI_CLASS_BRIDGE_IB_PCI), "infiniband", NULL }, > >> + { 0xFF, NULL, NULL }, > >> +}; > >> + > >> +static const PCISubClass comm_subclass[] = { > >> + { SUBCLASS(PCI_CLASS_COMMUNICATION_SERIAL), "serial", NULL }, > >> + { SUBCLASS(PCI_CLASS_COMMUNICATION_PARALLEL), "parallel", NULL }, > >> + { SUBCLASS(PCI_CLASS_COMMUNICATION_MULTISERIAL), "multiport-serial", NULL }, > >> + { SUBCLASS(PCI_CLASS_COMMUNICATION_MODEM), "modem", NULL }, > >> + { SUBCLASS(PCI_CLASS_COMMUNICATION_GPIB), "gpib", NULL }, > >> + { SUBCLASS(PCI_CLASS_COMMUNICATION_SC), "smart-card", NULL }, > >> + { 0xFF, NULL, NULL, NULL }, > >> +}; > >> + > >> +static const PCIIFace pic_iface[] = { > >> + { IFACE(PCI_CLASS_SYSTEM_PIC_IOAPIC), "io-apic" }, > >> + { IFACE(PCI_CLASS_SYSTEM_PIC_IOXAPIC), "io-xapic" }, > >> + { 0xFF, NULL }, > >> +}; > >> + > >> +static const PCISubClass sys_subclass[] = { > >> + { SUBCLASS(PCI_CLASS_SYSTEM_PIC), "interrupt-controller", pic_iface }, > >> + { SUBCLASS(PCI_CLASS_SYSTEM_DMA), "dma-controller", NULL }, > >> + { SUBCLASS(PCI_CLASS_SYSTEM_TIMER), "timer", NULL }, > >> + { SUBCLASS(PCI_CLASS_SYSTEM_RTC), "rtc", NULL }, > >> + { SUBCLASS(PCI_CLASS_SYSTEM_PCI_HOTPLUG), "hot-plug-controller", NULL }, > >> + { SUBCLASS(PCI_CLASS_SYSTEM_SDHCI), "sd-host-controller", NULL }, > >> + { 0xFF, NULL, NULL }, > >> +}; > >> + > >> +static const PCISubClass inp_subclass[] = { > >> + { SUBCLASS(PCI_CLASS_INPUT_KEYBOARD), "keyboard", NULL }, > >> + { SUBCLASS(PCI_CLASS_INPUT_PEN), "pen", NULL }, > >> + { SUBCLASS(PCI_CLASS_INPUT_MOUSE), "mouse", NULL }, > >> + { SUBCLASS(PCI_CLASS_INPUT_SCANNER), "scanner", NULL }, > >> + { SUBCLASS(PCI_CLASS_INPUT_GAMEPORT), "gameport", NULL }, > >> + { 0xFF, NULL, NULL }, > >> +}; > >> + > >> +static const PCISubClass dock_subclass[] = { > >> + { SUBCLASS(PCI_CLASS_DOCKING_GENERIC), "dock", NULL }, > >> + { 0xFF, NULL, NULL }, > >> +}; > >> + > >> +static const PCISubClass cpu_subclass[] = { > >> + { SUBCLASS(PCI_CLASS_PROCESSOR_PENTIUM), "pentium", NULL }, > >> + { SUBCLASS(PCI_CLASS_PROCESSOR_POWERPC), "powerpc", NULL }, > >> + { SUBCLASS(PCI_CLASS_PROCESSOR_MIPS), "mips", NULL }, > >> + { SUBCLASS(PCI_CLASS_PROCESSOR_CO), "co-processor", NULL }, > >> + { 0xFF, NULL, NULL }, > >> +}; > >> + > >> +static const PCIIFace usb_iface[] = { > >> + { IFACE(PCI_CLASS_SERIAL_USB_UHCI), "usb-uhci" }, > >> + { IFACE(PCI_CLASS_SERIAL_USB_OHCI), "usb-ohci", }, > >> + { IFACE(PCI_CLASS_SERIAL_USB_EHCI), "usb-ehci" }, > >> + { IFACE(PCI_CLASS_SERIAL_USB_XHCI), "usb-xhci" }, > >> + { IFACE(PCI_CLASS_SERIAL_USB_UNKNOWN), "usb-unknown" }, > >> + { IFACE(PCI_CLASS_SERIAL_USB_DEVICE), "usb-device" }, > >> + { 0xFF, NULL }, > >> +}; > >> + > >> +static const PCISubClass ser_subclass[] = { > >> + { SUBCLASS(PCI_CLASS_SERIAL_FIREWIRE), "firewire", NULL }, > >> + { SUBCLASS(PCI_CLASS_SERIAL_ACCESS), "access-bus", NULL }, > >> + { SUBCLASS(PCI_CLASS_SERIAL_SSA), "ssa", NULL }, > >> + { SUBCLASS(PCI_CLASS_SERIAL_USB), "usb", usb_iface }, > >> + { SUBCLASS(PCI_CLASS_SERIAL_FIBER), "fibre-channel", NULL }, > >> + { SUBCLASS(PCI_CLASS_SERIAL_SMBUS), "smb", NULL }, > >> + { SUBCLASS(PCI_CLASS_SERIAL_IB), "infiniband", NULL }, > >> + { SUBCLASS(PCI_CLASS_SERIAL_IPMI), "ipmi", NULL }, > >> + { SUBCLASS(PCI_CLASS_SERIAL_SERCOS), "sercos", NULL }, > >> + { SUBCLASS(PCI_CLASS_SERIAL_CANBUS), "canbus", NULL }, > >> + { 0xFF, NULL, NULL }, > >> +}; > >> + > >> +static const PCISubClass wrl_subclass[] = { > >> + { SUBCLASS(PCI_CLASS_WIRELESS_IRDA), "irda", NULL }, > >> + { SUBCLASS(PCI_CLASS_WIRELESS_CIR), "consumer-ir", NULL }, > >> + { SUBCLASS(PCI_CLASS_WIRELESS_RF_CONTROLLER), "rf-controller", NULL }, > >> + { SUBCLASS(PCI_CLASS_WIRELESS_BLUETOOTH), "bluetooth", NULL }, > >> + { SUBCLASS(PCI_CLASS_WIRELESS_BROADBAND), "broadband", NULL }, > >> + { 0xFF, NULL, NULL }, > >> +}; > >> + > >> +static const PCISubClass sat_subclass[] = { > >> + { SUBCLASS(PCI_CLASS_SATELLITE_TV), "satellite-tv", NULL }, > >> + { SUBCLASS(PCI_CLASS_SATELLITE_AUDIO), "satellite-audio", NULL }, > >> + { SUBCLASS(PCI_CLASS_SATELLITE_VOICE), "satellite-voice", NULL }, > >> + { SUBCLASS(PCI_CLASS_SATELLITE_DATA), "satellite-data", NULL }, > >> + { 0xFF, NULL, NULL }, > >> +}; > >> + > >> +static const PCISubClass crypt_subclass[] = { > >> + { SUBCLASS(PCI_CLASS_CRYPT_NETWORK), "network-encryption", NULL }, > >> + { SUBCLASS(PCI_CLASS_CRYPT_ENTERTAINMENT), > >> + "entertainment-encryption", NULL }, > >> + { 0xFF, NULL, NULL }, > >> +}; > >> + > >> +static const PCISubClass spc_subclass[] = { > >> + { SUBCLASS(PCI_CLASS_SP_DPIO), "dpio", NULL }, > >> + { SUBCLASS(PCI_CLASS_SP_PERF), "counter", NULL }, > >> + { SUBCLASS(PCI_CLASS_SP_SYNCH), "measurement", NULL }, > >> + { SUBCLASS(PCI_CLASS_SP_MANAGEMENT), "management-card", NULL }, > >> + { 0xFF, NULL, NULL }, > >> +}; > >> + > >> +static const PCIClass pci_classes[] = { > >> + { "legacy-device", undef_subclass }, > >> + { "mass-storage", mass_subclass }, > >> + { "network", net_subclass }, > >> + { "display", displ_subclass, }, > >> + { "multimedia-device", media_subclass }, > >> + { "memory-controller", mem_subclass }, > >> + { "unknown-bridge", bridg_subclass }, > >> + { "communication-controller", comm_subclass}, > >> + { "system-peripheral", sys_subclass }, > >> + { "input-controller", inp_subclass }, > >> + { "docking-station", dock_subclass }, > >> + { "cpu", cpu_subclass }, > >> + { "serial-bus", ser_subclass }, > >> + { "wireless-controller", wrl_subclass }, > >> + { "intelligent-io", NULL }, > >> + { "satellite-device", sat_subclass }, > >> + { "encryption", crypt_subclass }, > >> + { "data-processing-controller", spc_subclass }, > >> +}; > >> + > >> +static const char *pci_find_device_name(uint8_t class, uint8_t subclass, > >> + uint8_t iface) > >> +{ > >> + const PCIClass *pclass; > >> + const PCISubClass *psubclass; > >> + const PCIIFace *piface; > >> + const char *name; > >> + > >> + if (class >= ARRAY_SIZE(pci_classes)) { > >> + return "pci"; > >> + } > >> + > >> + pclass = pci_classes + class; > >> + name = pclass->name; > >> + > >> + if (pclass->subc == NULL) { > >> + return name; > >> + } > >> + > >> + psubclass = pclass->subc; > >> + while (psubclass->subclass != 0xff) { > >> + if (psubclass->subclass == subclass) { > >> + name = psubclass->name; > >> + break; > >> + } > >> + psubclass++; > >> + } > >> + > >> + piface = psubclass->iface; > >> + if (piface == NULL) { > >> + return name; > >> + } > >> + while (piface->iface != 0xff) { > >> + if (piface->iface == iface) { > >> + name = piface->name; > >> + break; > >> + } > >> + piface++; > >> + } > >> + > >> + return name; > >> +} > >> + > >> +static void pci_get_node_name(char *nodename, int len, PCIDevice *dev) > >> +{ > >> + int slot = PCI_SLOT(dev->devfn); > >> + int func = PCI_FUNC(dev->devfn); > >> + uint32_t ccode = pci_default_read_config(dev, PCI_CLASS_PROG, 3); > >> + const char *name; > >> + > >> + name = pci_find_device_name((ccode >> 16) & 0xff, (ccode >> 8) & 0xff, > >> + ccode & 0xff); > >> + > >> + if (func != 0) { > >> + snprintf(nodename, len, "%s@%x,%x", name, slot, func); > >> + } else { > >> + snprintf(nodename, len, "%s@%x", name, slot); > >> + } > >> +} > >> + > >> static uint32_t spapr_phb_get_pci_drc_index(sPAPRPHBState *phb, > >> PCIDevice *pdev); > >> > >> @@ -955,6 +1226,7 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset, > >> int pci_status, err; > >> char *buf = NULL; > >> uint32_t drc_index = spapr_phb_get_pci_drc_index(sphb, dev); > >> + uint32_t ccode = pci_default_read_config(dev, PCI_CLASS_PROG, 3); > >> > >> if (pci_default_read_config(dev, PCI_HEADER_TYPE, 1) == > >> PCI_HEADER_TYPE_BRIDGE) { > >> @@ -968,8 +1240,7 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset, > >> pci_default_read_config(dev, PCI_DEVICE_ID, 2))); > >> _FDT(fdt_setprop_cell(fdt, offset, "revision-id", > >> pci_default_read_config(dev, PCI_REVISION_ID, 1))); > >> - _FDT(fdt_setprop_cell(fdt, offset, "class-code", > >> - pci_default_read_config(dev, PCI_CLASS_PROG, 3))); > >> + _FDT(fdt_setprop_cell(fdt, offset, "class-code", ccode)); > >> if (pci_default_read_config(dev, PCI_INTERRUPT_PIN, 1)) { > >> _FDT(fdt_setprop_cell(fdt, offset, "interrupts", > >> pci_default_read_config(dev, PCI_INTERRUPT_PIN, 1))); > >> @@ -1010,11 +1281,10 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset, > >> _FDT(fdt_setprop(fdt, offset, "udf-supported", NULL, 0)); > >> } > >> > >> - /* NOTE: this is normally generated by firmware via path/unit name, > >> - * but in our case we must set it manually since it does not get > >> - * processed by OF beforehand > >> - */ > >> - _FDT(fdt_setprop_string(fdt, offset, "name", "pci")); > >> + _FDT(fdt_setprop_string(fdt, offset, "name", > >> + pci_find_device_name((ccode >> 16) & 0xff, > >> + (ccode >> 8) & 0xff, > >> + ccode & 0xff))); > >> buf = spapr_phb_get_loc_code(sphb, dev); > >> if (!buf) { > >> error_report("Failed setting the ibm,loc-code"); > >> @@ -1051,15 +1321,9 @@ static int spapr_create_pci_child_dt(sPAPRPHBState *phb, PCIDevice *dev, > >> void *fdt, int node_offset) > >> { > >> int offset, ret; > >> - int slot = PCI_SLOT(dev->devfn); > >> - int func = PCI_FUNC(dev->devfn); > >> char nodename[FDT_NAME_MAX]; > >> > >> - if (func != 0) { > >> - snprintf(nodename, FDT_NAME_MAX, "pci@%x,%x", slot, func); > >> - } else { > >> - snprintf(nodename, FDT_NAME_MAX, "pci@%x", slot); > >> - } > >> + pci_get_node_name(nodename, FDT_NAME_MAX, dev); > >> offset = fdt_add_subnode(fdt, node_offset, nodename); > >> ret = spapr_populate_pci_child_dt(dev, fdt, offset, phb); > >> > >> -- > >> 2.4.3 > >> > >> > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/2] spapr: generate DT node names 2015-09-25 9:12 ` Michael S. Tsirkin @ 2015-09-25 10:21 ` Laurent Vivier 0 siblings, 0 replies; 12+ messages in thread From: Laurent Vivier @ 2015-09-25 10:21 UTC (permalink / raw) To: Michael S. Tsirkin Cc: thuth, qemu-devel, Alexander Graf, Gavin Shan, qemu-ppc, David Gibson On 25/09/2015 11:12, Michael S. Tsirkin wrote: > On Fri, Sep 25, 2015 at 10:29:41AM +0200, Laurent Vivier wrote: >> >> >> On 25/09/2015 01:29, Gavin Shan wrote: >>> On Thu, Sep 24, 2015 at 12:27:39PM +0200, Laurent Vivier wrote: >>>> When DT node names for PCI devices are generated by SLOF, >>>> they are generated according to the type of the device >>>> (for instance, ethernet for virtio-net-pci device). >>>> >>>> Node name for hotplugged devices is generated by QEMU. >>>> This patch adds the mechanic to QEMU to create the node >>>> name according to the device type too. >>>> >>>> The data structure has been roughly copied from OpenBIOS/OpenHackware, >>>> node names from SLOF. >>>> >>>> Example: >>>> >>>> Hotplugging some PCI cards with QEMU monitor: >>>> >>>> device_add virtio-tablet-pci >>>> device_add virtio-serial-pci >>>> device_add virtio-mouse-pci >>>> device_add virtio-scsi-pci >>>> device_add virtio-gpu-pci >>>> device_add ne2k_pci >>>> device_add nec-usb-xhci >>>> device_add intel-hda >>>> >>>> What we can see in linux device tree: >>>> >>>> for dir in /proc/device-tree/pci@800000020000000/*@*/; do >>>> echo $dir >>>> cat $dir/name >>>> echo >>>> done >>>> >>>> WITHOUT this patch: >>>> >>>> /proc/device-tree/pci@800000020000000/pci@0/ >>>> pci >>>> /proc/device-tree/pci@800000020000000/pci@1/ >>>> pci >>>> /proc/device-tree/pci@800000020000000/pci@2/ >>>> pci >>>> /proc/device-tree/pci@800000020000000/pci@3/ >>>> pci >>>> /proc/device-tree/pci@800000020000000/pci@4/ >>>> pci >>>> /proc/device-tree/pci@800000020000000/pci@5/ >>>> pci >>>> /proc/device-tree/pci@800000020000000/pci@6/ >>>> pci >>>> /proc/device-tree/pci@800000020000000/pci@7/ >>>> pci >>>> >>>> WITH this patch: >>>> >>>> /proc/device-tree/pci@800000020000000/communication-controller@1/ >>>> communication-controller >>>> /proc/device-tree/pci@800000020000000/display@4/ >>>> display >>>> /proc/device-tree/pci@800000020000000/ethernet@5/ >>>> ethernet >>>> /proc/device-tree/pci@800000020000000/input-controller@0/ >>>> input-controller >>>> /proc/device-tree/pci@800000020000000/mouse@2/ >>>> mouse >>>> /proc/device-tree/pci@800000020000000/multimedia-device@7/ >>>> multimedia-device >>>> /proc/device-tree/pci@800000020000000/scsi@3/ >>>> scsi >>>> /proc/device-tree/pci@800000020000000/usb-xhci@6/ >>>> usb-xhci >>>> >>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com> >>>> Reviewed-by: Thomas Huth <thuth@redhat.com> >>>> --- >>>> hw/ppc/spapr_pci.c | 292 ++++++++++++++++++++++++++++++++++++++++++++++++++--- >>>> 1 file changed, 278 insertions(+), 14 deletions(-) >>>> >>>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c >>>> index a2feb4c..63eb28c 100644 >>>> --- a/hw/ppc/spapr_pci.c >>>> +++ b/hw/ppc/spapr_pci.c >>>> @@ -38,6 +38,7 @@ >>>> >>>> #include "hw/pci/pci_bridge.h" >>>> #include "hw/pci/pci_bus.h" >>>> +#include "hw/pci/pci_ids.h" >>>> #include "hw/ppc/spapr_drc.h" >>>> #include "sysemu/device_tree.h" >>>> >>>> @@ -944,6 +945,276 @@ static void populate_resource_props(PCIDevice *d, ResourceProps *rp) >>>> rp->assigned_len = assigned_idx * sizeof(ResourceFields); >>>> } >>>> >>> >>> One question would be: is there one reason why the logic, converting >>> class/subclass/iface code to tring, isn't put into generic PCI module? >>> If the code is put there, all platforms can reuse it. >> >> For the moment, it is only used by the device tree generation for spapr, >> and moreover the names are ones from the openfirmware specification, so >> except if openbios on sparc/macintosh takes its device tree from QEMU I >> see no reason to put this in a generic PCI module. >> >> Laurent > > Our boot paths are supposed to use OF compatible names. > I note they look different for some reason. Do you want I move this code to the PCI directory (pci_database.c, for instance) ? It's up to you. >>> >>> Thanks, >>> Gavin >>> >>>> +typedef struct PCIClass PCIClass; >>>> +typedef struct PCISubClass PCISubClass; >>>> +typedef struct PCIIFace PCIIFace; >>>> + >>>> +struct PCIIFace { >>>> + uint8_t iface; >>>> + const char *name; >>>> +}; >>>> + >>>> +struct PCISubClass { >>>> + uint8_t subclass; >>>> + const char *name; >>>> + const PCIIFace *iface; >>>> +}; >>>> +#define SUBCLASS(a) ((uint8_t)a) >>>> +#define IFACE(a) ((uint8_t)a) >>>> + >>>> +struct PCIClass { >>>> + const char *name; >>>> + const PCISubClass *subc; >>>> +}; >>>> + >>>> +static const PCISubClass undef_subclass[] = { >>>> + { IFACE(PCI_CLASS_NOT_DEFINED_VGA), "display", NULL }, >>>> + { 0xFF, NULL, NULL, NULL }, >>>> +}; >>>> + >>>> +static const PCISubClass mass_subclass[] = { >>>> + { SUBCLASS(PCI_CLASS_STORAGE_SCSI), "scsi", NULL }, >>>> + { SUBCLASS(PCI_CLASS_STORAGE_IDE), "ide", NULL }, >>>> + { SUBCLASS(PCI_CLASS_STORAGE_FLOPPY), "fdc", NULL }, >>>> + { SUBCLASS(PCI_CLASS_STORAGE_IPI), "ipi", NULL }, >>>> + { SUBCLASS(PCI_CLASS_STORAGE_RAID), "raid", NULL }, >>>> + { SUBCLASS(PCI_CLASS_STORAGE_ATA), "ata", NULL }, >>>> + { SUBCLASS(PCI_CLASS_STORAGE_SATA), "sata", NULL }, >>>> + { SUBCLASS(PCI_CLASS_STORAGE_SAS), "sas", NULL }, >>>> + { 0xFF, NULL, NULL }, >>>> +}; >>>> + >>>> +static const PCISubClass net_subclass[] = { >>>> + { SUBCLASS(PCI_CLASS_NETWORK_ETHERNET), "ethernet", NULL }, >>>> + { SUBCLASS(PCI_CLASS_NETWORK_TOKEN_RING), "token-ring", NULL }, >>>> + { SUBCLASS(PCI_CLASS_NETWORK_FDDI), "fddi", NULL }, >>>> + { SUBCLASS(PCI_CLASS_NETWORK_ATM), "atm", NULL }, >>>> + { SUBCLASS(PCI_CLASS_NETWORK_ISDN), "isdn", NULL }, >>>> + { SUBCLASS(PCI_CLASS_NETWORK_WORLDFIP), "worldfip", NULL }, >>>> + { SUBCLASS(PCI_CLASS_NETWORK_PICMG214), "picmg", NULL }, >>>> + { 0xFF, NULL, NULL }, >>>> +}; >>>> + >>>> +static const PCISubClass displ_subclass[] = { >>>> + { SUBCLASS(PCI_CLASS_DISPLAY_VGA), "vga", NULL }, >>>> + { SUBCLASS(PCI_CLASS_DISPLAY_XGA), "xga", NULL }, >>>> + { SUBCLASS(PCI_CLASS_DISPLAY_3D), "3d-controller", NULL }, >>>> + { 0xFF, NULL, NULL }, >>>> +}; >>>> + >>>> +static const PCISubClass media_subclass[] = { >>>> + { SUBCLASS(PCI_CLASS_MULTIMEDIA_VIDEO), "video", NULL }, >>>> + { SUBCLASS(PCI_CLASS_MULTIMEDIA_AUDIO), "sound", NULL }, >>>> + { SUBCLASS(PCI_CLASS_MULTIMEDIA_PHONE), "telephony", NULL }, >>>> + { 0xFF, NULL, NULL }, >>>> +}; >>>> + >>>> +static const PCISubClass mem_subclass[] = { >>>> + { SUBCLASS(PCI_CLASS_MEMORY_RAM), "memory", NULL }, >>>> + { SUBCLASS(PCI_CLASS_MEMORY_FLASH), "flash", NULL }, >>>> + { 0xFF, NULL, NULL }, >>>> +}; >>>> + >>>> +static const PCISubClass bridg_subclass[] = { >>>> + { SUBCLASS(PCI_CLASS_BRIDGE_HOST), "host", NULL }, >>>> + { SUBCLASS(PCI_CLASS_BRIDGE_ISA), "isa", NULL }, >>>> + { SUBCLASS(PCI_CLASS_BRIDGE_EISA), "eisa", NULL }, >>>> + { SUBCLASS(PCI_CLASS_BRIDGE_MC), "mca", NULL }, >>>> + { SUBCLASS(PCI_CLASS_BRIDGE_PCI), "pci", NULL }, >>>> + { SUBCLASS(PCI_CLASS_BRIDGE_PCMCIA), "pcmcia", NULL }, >>>> + { SUBCLASS(PCI_CLASS_BRIDGE_NUBUS), "nubus", NULL }, >>>> + { SUBCLASS(PCI_CLASS_BRIDGE_CARDBUS), "cardbus", NULL }, >>>> + { SUBCLASS(PCI_CLASS_BRIDGE_RACEWAY), "raceway", NULL }, >>>> + { SUBCLASS(PCI_CLASS_BRIDGE_PCI_SEMITP), "semi-transparent-pci", NULL }, >>>> + { SUBCLASS(PCI_CLASS_BRIDGE_IB_PCI), "infiniband", NULL }, >>>> + { 0xFF, NULL, NULL }, >>>> +}; >>>> + >>>> +static const PCISubClass comm_subclass[] = { >>>> + { SUBCLASS(PCI_CLASS_COMMUNICATION_SERIAL), "serial", NULL }, >>>> + { SUBCLASS(PCI_CLASS_COMMUNICATION_PARALLEL), "parallel", NULL }, >>>> + { SUBCLASS(PCI_CLASS_COMMUNICATION_MULTISERIAL), "multiport-serial", NULL }, >>>> + { SUBCLASS(PCI_CLASS_COMMUNICATION_MODEM), "modem", NULL }, >>>> + { SUBCLASS(PCI_CLASS_COMMUNICATION_GPIB), "gpib", NULL }, >>>> + { SUBCLASS(PCI_CLASS_COMMUNICATION_SC), "smart-card", NULL }, >>>> + { 0xFF, NULL, NULL, NULL }, >>>> +}; >>>> + >>>> +static const PCIIFace pic_iface[] = { >>>> + { IFACE(PCI_CLASS_SYSTEM_PIC_IOAPIC), "io-apic" }, >>>> + { IFACE(PCI_CLASS_SYSTEM_PIC_IOXAPIC), "io-xapic" }, >>>> + { 0xFF, NULL }, >>>> +}; >>>> + >>>> +static const PCISubClass sys_subclass[] = { >>>> + { SUBCLASS(PCI_CLASS_SYSTEM_PIC), "interrupt-controller", pic_iface }, >>>> + { SUBCLASS(PCI_CLASS_SYSTEM_DMA), "dma-controller", NULL }, >>>> + { SUBCLASS(PCI_CLASS_SYSTEM_TIMER), "timer", NULL }, >>>> + { SUBCLASS(PCI_CLASS_SYSTEM_RTC), "rtc", NULL }, >>>> + { SUBCLASS(PCI_CLASS_SYSTEM_PCI_HOTPLUG), "hot-plug-controller", NULL }, >>>> + { SUBCLASS(PCI_CLASS_SYSTEM_SDHCI), "sd-host-controller", NULL }, >>>> + { 0xFF, NULL, NULL }, >>>> +}; >>>> + >>>> +static const PCISubClass inp_subclass[] = { >>>> + { SUBCLASS(PCI_CLASS_INPUT_KEYBOARD), "keyboard", NULL }, >>>> + { SUBCLASS(PCI_CLASS_INPUT_PEN), "pen", NULL }, >>>> + { SUBCLASS(PCI_CLASS_INPUT_MOUSE), "mouse", NULL }, >>>> + { SUBCLASS(PCI_CLASS_INPUT_SCANNER), "scanner", NULL }, >>>> + { SUBCLASS(PCI_CLASS_INPUT_GAMEPORT), "gameport", NULL }, >>>> + { 0xFF, NULL, NULL }, >>>> +}; >>>> + >>>> +static const PCISubClass dock_subclass[] = { >>>> + { SUBCLASS(PCI_CLASS_DOCKING_GENERIC), "dock", NULL }, >>>> + { 0xFF, NULL, NULL }, >>>> +}; >>>> + >>>> +static const PCISubClass cpu_subclass[] = { >>>> + { SUBCLASS(PCI_CLASS_PROCESSOR_PENTIUM), "pentium", NULL }, >>>> + { SUBCLASS(PCI_CLASS_PROCESSOR_POWERPC), "powerpc", NULL }, >>>> + { SUBCLASS(PCI_CLASS_PROCESSOR_MIPS), "mips", NULL }, >>>> + { SUBCLASS(PCI_CLASS_PROCESSOR_CO), "co-processor", NULL }, >>>> + { 0xFF, NULL, NULL }, >>>> +}; >>>> + >>>> +static const PCIIFace usb_iface[] = { >>>> + { IFACE(PCI_CLASS_SERIAL_USB_UHCI), "usb-uhci" }, >>>> + { IFACE(PCI_CLASS_SERIAL_USB_OHCI), "usb-ohci", }, >>>> + { IFACE(PCI_CLASS_SERIAL_USB_EHCI), "usb-ehci" }, >>>> + { IFACE(PCI_CLASS_SERIAL_USB_XHCI), "usb-xhci" }, >>>> + { IFACE(PCI_CLASS_SERIAL_USB_UNKNOWN), "usb-unknown" }, >>>> + { IFACE(PCI_CLASS_SERIAL_USB_DEVICE), "usb-device" }, >>>> + { 0xFF, NULL }, >>>> +}; >>>> + >>>> +static const PCISubClass ser_subclass[] = { >>>> + { SUBCLASS(PCI_CLASS_SERIAL_FIREWIRE), "firewire", NULL }, >>>> + { SUBCLASS(PCI_CLASS_SERIAL_ACCESS), "access-bus", NULL }, >>>> + { SUBCLASS(PCI_CLASS_SERIAL_SSA), "ssa", NULL }, >>>> + { SUBCLASS(PCI_CLASS_SERIAL_USB), "usb", usb_iface }, >>>> + { SUBCLASS(PCI_CLASS_SERIAL_FIBER), "fibre-channel", NULL }, >>>> + { SUBCLASS(PCI_CLASS_SERIAL_SMBUS), "smb", NULL }, >>>> + { SUBCLASS(PCI_CLASS_SERIAL_IB), "infiniband", NULL }, >>>> + { SUBCLASS(PCI_CLASS_SERIAL_IPMI), "ipmi", NULL }, >>>> + { SUBCLASS(PCI_CLASS_SERIAL_SERCOS), "sercos", NULL }, >>>> + { SUBCLASS(PCI_CLASS_SERIAL_CANBUS), "canbus", NULL }, >>>> + { 0xFF, NULL, NULL }, >>>> +}; >>>> + >>>> +static const PCISubClass wrl_subclass[] = { >>>> + { SUBCLASS(PCI_CLASS_WIRELESS_IRDA), "irda", NULL }, >>>> + { SUBCLASS(PCI_CLASS_WIRELESS_CIR), "consumer-ir", NULL }, >>>> + { SUBCLASS(PCI_CLASS_WIRELESS_RF_CONTROLLER), "rf-controller", NULL }, >>>> + { SUBCLASS(PCI_CLASS_WIRELESS_BLUETOOTH), "bluetooth", NULL }, >>>> + { SUBCLASS(PCI_CLASS_WIRELESS_BROADBAND), "broadband", NULL }, >>>> + { 0xFF, NULL, NULL }, >>>> +}; >>>> + >>>> +static const PCISubClass sat_subclass[] = { >>>> + { SUBCLASS(PCI_CLASS_SATELLITE_TV), "satellite-tv", NULL }, >>>> + { SUBCLASS(PCI_CLASS_SATELLITE_AUDIO), "satellite-audio", NULL }, >>>> + { SUBCLASS(PCI_CLASS_SATELLITE_VOICE), "satellite-voice", NULL }, >>>> + { SUBCLASS(PCI_CLASS_SATELLITE_DATA), "satellite-data", NULL }, >>>> + { 0xFF, NULL, NULL }, >>>> +}; >>>> + >>>> +static const PCISubClass crypt_subclass[] = { >>>> + { SUBCLASS(PCI_CLASS_CRYPT_NETWORK), "network-encryption", NULL }, >>>> + { SUBCLASS(PCI_CLASS_CRYPT_ENTERTAINMENT), >>>> + "entertainment-encryption", NULL }, >>>> + { 0xFF, NULL, NULL }, >>>> +}; >>>> + >>>> +static const PCISubClass spc_subclass[] = { >>>> + { SUBCLASS(PCI_CLASS_SP_DPIO), "dpio", NULL }, >>>> + { SUBCLASS(PCI_CLASS_SP_PERF), "counter", NULL }, >>>> + { SUBCLASS(PCI_CLASS_SP_SYNCH), "measurement", NULL }, >>>> + { SUBCLASS(PCI_CLASS_SP_MANAGEMENT), "management-card", NULL }, >>>> + { 0xFF, NULL, NULL }, >>>> +}; >>>> + >>>> +static const PCIClass pci_classes[] = { >>>> + { "legacy-device", undef_subclass }, >>>> + { "mass-storage", mass_subclass }, >>>> + { "network", net_subclass }, >>>> + { "display", displ_subclass, }, >>>> + { "multimedia-device", media_subclass }, >>>> + { "memory-controller", mem_subclass }, >>>> + { "unknown-bridge", bridg_subclass }, >>>> + { "communication-controller", comm_subclass}, >>>> + { "system-peripheral", sys_subclass }, >>>> + { "input-controller", inp_subclass }, >>>> + { "docking-station", dock_subclass }, >>>> + { "cpu", cpu_subclass }, >>>> + { "serial-bus", ser_subclass }, >>>> + { "wireless-controller", wrl_subclass }, >>>> + { "intelligent-io", NULL }, >>>> + { "satellite-device", sat_subclass }, >>>> + { "encryption", crypt_subclass }, >>>> + { "data-processing-controller", spc_subclass }, >>>> +}; >>>> + >>>> +static const char *pci_find_device_name(uint8_t class, uint8_t subclass, >>>> + uint8_t iface) >>>> +{ >>>> + const PCIClass *pclass; >>>> + const PCISubClass *psubclass; >>>> + const PCIIFace *piface; >>>> + const char *name; >>>> + >>>> + if (class >= ARRAY_SIZE(pci_classes)) { >>>> + return "pci"; >>>> + } >>>> + >>>> + pclass = pci_classes + class; >>>> + name = pclass->name; >>>> + >>>> + if (pclass->subc == NULL) { >>>> + return name; >>>> + } >>>> + >>>> + psubclass = pclass->subc; >>>> + while (psubclass->subclass != 0xff) { >>>> + if (psubclass->subclass == subclass) { >>>> + name = psubclass->name; >>>> + break; >>>> + } >>>> + psubclass++; >>>> + } >>>> + >>>> + piface = psubclass->iface; >>>> + if (piface == NULL) { >>>> + return name; >>>> + } >>>> + while (piface->iface != 0xff) { >>>> + if (piface->iface == iface) { >>>> + name = piface->name; >>>> + break; >>>> + } >>>> + piface++; >>>> + } >>>> + >>>> + return name; >>>> +} >>>> + >>>> +static void pci_get_node_name(char *nodename, int len, PCIDevice *dev) >>>> +{ >>>> + int slot = PCI_SLOT(dev->devfn); >>>> + int func = PCI_FUNC(dev->devfn); >>>> + uint32_t ccode = pci_default_read_config(dev, PCI_CLASS_PROG, 3); >>>> + const char *name; >>>> + >>>> + name = pci_find_device_name((ccode >> 16) & 0xff, (ccode >> 8) & 0xff, >>>> + ccode & 0xff); >>>> + >>>> + if (func != 0) { >>>> + snprintf(nodename, len, "%s@%x,%x", name, slot, func); >>>> + } else { >>>> + snprintf(nodename, len, "%s@%x", name, slot); >>>> + } >>>> +} >>>> + >>>> static uint32_t spapr_phb_get_pci_drc_index(sPAPRPHBState *phb, >>>> PCIDevice *pdev); >>>> >>>> @@ -955,6 +1226,7 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset, >>>> int pci_status, err; >>>> char *buf = NULL; >>>> uint32_t drc_index = spapr_phb_get_pci_drc_index(sphb, dev); >>>> + uint32_t ccode = pci_default_read_config(dev, PCI_CLASS_PROG, 3); >>>> >>>> if (pci_default_read_config(dev, PCI_HEADER_TYPE, 1) == >>>> PCI_HEADER_TYPE_BRIDGE) { >>>> @@ -968,8 +1240,7 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset, >>>> pci_default_read_config(dev, PCI_DEVICE_ID, 2))); >>>> _FDT(fdt_setprop_cell(fdt, offset, "revision-id", >>>> pci_default_read_config(dev, PCI_REVISION_ID, 1))); >>>> - _FDT(fdt_setprop_cell(fdt, offset, "class-code", >>>> - pci_default_read_config(dev, PCI_CLASS_PROG, 3))); >>>> + _FDT(fdt_setprop_cell(fdt, offset, "class-code", ccode)); >>>> if (pci_default_read_config(dev, PCI_INTERRUPT_PIN, 1)) { >>>> _FDT(fdt_setprop_cell(fdt, offset, "interrupts", >>>> pci_default_read_config(dev, PCI_INTERRUPT_PIN, 1))); >>>> @@ -1010,11 +1281,10 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset, >>>> _FDT(fdt_setprop(fdt, offset, "udf-supported", NULL, 0)); >>>> } >>>> >>>> - /* NOTE: this is normally generated by firmware via path/unit name, >>>> - * but in our case we must set it manually since it does not get >>>> - * processed by OF beforehand >>>> - */ >>>> - _FDT(fdt_setprop_string(fdt, offset, "name", "pci")); >>>> + _FDT(fdt_setprop_string(fdt, offset, "name", >>>> + pci_find_device_name((ccode >> 16) & 0xff, >>>> + (ccode >> 8) & 0xff, >>>> + ccode & 0xff))); >>>> buf = spapr_phb_get_loc_code(sphb, dev); >>>> if (!buf) { >>>> error_report("Failed setting the ibm,loc-code"); >>>> @@ -1051,15 +1321,9 @@ static int spapr_create_pci_child_dt(sPAPRPHBState *phb, PCIDevice *dev, >>>> void *fdt, int node_offset) >>>> { >>>> int offset, ret; >>>> - int slot = PCI_SLOT(dev->devfn); >>>> - int func = PCI_FUNC(dev->devfn); >>>> char nodename[FDT_NAME_MAX]; >>>> >>>> - if (func != 0) { >>>> - snprintf(nodename, FDT_NAME_MAX, "pci@%x,%x", slot, func); >>>> - } else { >>>> - snprintf(nodename, FDT_NAME_MAX, "pci@%x", slot); >>>> - } >>>> + pci_get_node_name(nodename, FDT_NAME_MAX, dev); >>>> offset = fdt_add_subnode(fdt, node_offset, nodename); >>>> ret = spapr_populate_pci_child_dt(dev, fdt, offset, phb); >>>> >>>> -- >>>> 2.4.3 >>>> >>>> >>> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/2] spapr: generate DT node names 2015-09-24 10:27 ` [Qemu-devel] [PATCH v4 2/2] spapr: generate DT node names Laurent Vivier 2015-09-24 23:29 ` Gavin Shan @ 2015-09-29 5:18 ` David Gibson 2015-09-29 8:37 ` Laurent Vivier 1 sibling, 1 reply; 12+ messages in thread From: David Gibson @ 2015-09-29 5:18 UTC (permalink / raw) To: Laurent Vivier Cc: thuth, Michael S. Tsirkin, qemu-ppc, Alexander Graf, qemu-devel [-- Attachment #1: Type: text/plain, Size: 8660 bytes --] On Thu, Sep 24, 2015 at 12:27:39PM +0200, Laurent Vivier wrote: > When DT node names for PCI devices are generated by SLOF, > they are generated according to the type of the device > (for instance, ethernet for virtio-net-pci device). > > Node name for hotplugged devices is generated by QEMU. > This patch adds the mechanic to QEMU to create the node > name according to the device type too. > > The data structure has been roughly copied from OpenBIOS/OpenHackware, > node names from SLOF. > > Example: > > Hotplugging some PCI cards with QEMU monitor: > > device_add virtio-tablet-pci > device_add virtio-serial-pci > device_add virtio-mouse-pci > device_add virtio-scsi-pci > device_add virtio-gpu-pci > device_add ne2k_pci > device_add nec-usb-xhci > device_add intel-hda > > What we can see in linux device tree: > > for dir in /proc/device-tree/pci@800000020000000/*@*/; do > echo $dir > cat $dir/name > echo > done > > WITHOUT this patch: > > /proc/device-tree/pci@800000020000000/pci@0/ > pci > /proc/device-tree/pci@800000020000000/pci@1/ > pci > /proc/device-tree/pci@800000020000000/pci@2/ > pci > /proc/device-tree/pci@800000020000000/pci@3/ > pci > /proc/device-tree/pci@800000020000000/pci@4/ > pci > /proc/device-tree/pci@800000020000000/pci@5/ > pci > /proc/device-tree/pci@800000020000000/pci@6/ > pci > /proc/device-tree/pci@800000020000000/pci@7/ > pci > > WITH this patch: > > /proc/device-tree/pci@800000020000000/communication-controller@1/ > communication-controller > /proc/device-tree/pci@800000020000000/display@4/ > display > /proc/device-tree/pci@800000020000000/ethernet@5/ > ethernet > /proc/device-tree/pci@800000020000000/input-controller@0/ > input-controller > /proc/device-tree/pci@800000020000000/mouse@2/ > mouse > /proc/device-tree/pci@800000020000000/multimedia-device@7/ > multimedia-device > /proc/device-tree/pci@800000020000000/scsi@3/ > scsi > /proc/device-tree/pci@800000020000000/usb-xhci@6/ > usb-xhci > > Signed-off-by: Laurent Vivier <lvivier@redhat.com> > Reviewed-by: Thomas Huth <thuth@redhat.com> Concept and approach seem good to me. > --- > hw/ppc/spapr_pci.c | 292 ++++++++++++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 278 insertions(+), 14 deletions(-) > > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index a2feb4c..63eb28c 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -38,6 +38,7 @@ > > #include "hw/pci/pci_bridge.h" > #include "hw/pci/pci_bus.h" > +#include "hw/pci/pci_ids.h" > #include "hw/ppc/spapr_drc.h" > #include "sysemu/device_tree.h" > > @@ -944,6 +945,276 @@ static void populate_resource_props(PCIDevice *d, ResourceProps *rp) > rp->assigned_len = assigned_idx * sizeof(ResourceFields); > } > > +typedef struct PCIClass PCIClass; > +typedef struct PCISubClass PCISubClass; > +typedef struct PCIIFace PCIIFace; > + > +struct PCIIFace { > + uint8_t iface; > + const char *name; > +}; > + > +struct PCISubClass { > + uint8_t subclass; > + const char *name; > + const PCIIFace *iface; > +}; > +#define SUBCLASS(a) ((uint8_t)a) > +#define IFACE(a) ((uint8_t)a) > + > +struct PCIClass { > + const char *name; > + const PCISubClass *subc; > +}; > + > +static const PCISubClass undef_subclass[] = { > + { IFACE(PCI_CLASS_NOT_DEFINED_VGA), "display", NULL }, These IFACE and SUBCLASS macro calls seem ugly to me. What's the purpose of them? [snip] > +static const char *pci_find_device_name(uint8_t class, uint8_t subclass, > + uint8_t iface) > +{ > + const PCIClass *pclass; > + const PCISubClass *psubclass; > + const PCIIFace *piface; > + const char *name; > + > + if (class >= ARRAY_SIZE(pci_classes)) { > + return "pci"; > + } > + > + pclass = pci_classes + class; > + name = pclass->name; > + > + if (pclass->subc == NULL) { > + return name; > + } > + > + psubclass = pclass->subc; > + while (psubclass->subclass != 0xff) { > + if (psubclass->subclass == subclass) { > + name = psubclass->name; > + break; > + } > + psubclass++; > + } > + > + piface = psubclass->iface; > + if (piface == NULL) { > + return name; > + } > + while (piface->iface != 0xff) { > + if (piface->iface == iface) { > + name = piface->name; > + break; > + } > + piface++; > + } > + > + return name; > +} > + > +static void pci_get_node_name(char *nodename, int len, PCIDevice *dev) > +{ > + int slot = PCI_SLOT(dev->devfn); > + int func = PCI_FUNC(dev->devfn); > + uint32_t ccode = pci_default_read_config(dev, PCI_CLASS_PROG, 3); > + const char *name; > + > + name = pci_find_device_name((ccode >> 16) & 0xff, (ccode >> 8) & 0xff, > + ccode & 0xff); > + > + if (func != 0) { > + snprintf(nodename, len, "%s@%x,%x", name, slot, func); > + } else { > + snprintf(nodename, len, "%s@%x", name, slot); > + } > +} > + > static uint32_t spapr_phb_get_pci_drc_index(sPAPRPHBState *phb, > PCIDevice *pdev); > > @@ -955,6 +1226,7 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset, > int pci_status, err; > char *buf = NULL; > uint32_t drc_index = spapr_phb_get_pci_drc_index(sphb, dev); > + uint32_t ccode = pci_default_read_config(dev, PCI_CLASS_PROG, 3); > > if (pci_default_read_config(dev, PCI_HEADER_TYPE, 1) == > PCI_HEADER_TYPE_BRIDGE) { > @@ -968,8 +1240,7 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset, > pci_default_read_config(dev, PCI_DEVICE_ID, 2))); > _FDT(fdt_setprop_cell(fdt, offset, "revision-id", > pci_default_read_config(dev, PCI_REVISION_ID, 1))); > - _FDT(fdt_setprop_cell(fdt, offset, "class-code", > - pci_default_read_config(dev, PCI_CLASS_PROG, 3))); > + _FDT(fdt_setprop_cell(fdt, offset, "class-code", ccode)); > if (pci_default_read_config(dev, PCI_INTERRUPT_PIN, 1)) { > _FDT(fdt_setprop_cell(fdt, offset, "interrupts", > pci_default_read_config(dev, PCI_INTERRUPT_PIN, 1))); > @@ -1010,11 +1281,10 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset, > _FDT(fdt_setprop(fdt, offset, "udf-supported", NULL, 0)); > } > > - /* NOTE: this is normally generated by firmware via path/unit name, > - * but in our case we must set it manually since it does not get > - * processed by OF beforehand > - */ > - _FDT(fdt_setprop_string(fdt, offset, "name", "pci")); > + _FDT(fdt_setprop_string(fdt, offset, "name", > + pci_find_device_name((ccode >> 16) & 0xff, > + (ccode >> 8) & 0xff, > + ccode & 0xff))); Heh. This isn't wrong, but there's a non-obvious wrinkle as to why. Generally flattened trees shouldn't require the 'name' property (the consumer is supposed to infer that from the node name). However, since this may also be used to generate tree fragments which get passed through the PAPR dynamic reconfiguration interface, and that *does* expect the name property to be passed through. > buf = spapr_phb_get_loc_code(sphb, dev); > if (!buf) { > error_report("Failed setting the ibm,loc-code"); > @@ -1051,15 +1321,9 @@ static int spapr_create_pci_child_dt(sPAPRPHBState *phb, PCIDevice *dev, > void *fdt, int node_offset) > { > int offset, ret; > - int slot = PCI_SLOT(dev->devfn); > - int func = PCI_FUNC(dev->devfn); > char nodename[FDT_NAME_MAX]; > > - if (func != 0) { > - snprintf(nodename, FDT_NAME_MAX, "pci@%x,%x", slot, func); > - } else { > - snprintf(nodename, FDT_NAME_MAX, "pci@%x", slot); > - } > + pci_get_node_name(nodename, FDT_NAME_MAX, dev); > offset = fdt_add_subnode(fdt, node_offset, nodename); > ret = spapr_populate_pci_child_dt(dev, fdt, offset, phb); > -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson [-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/2] spapr: generate DT node names 2015-09-29 5:18 ` David Gibson @ 2015-09-29 8:37 ` Laurent Vivier 2015-09-29 9:40 ` Thomas Huth 2015-09-30 4:33 ` David Gibson 0 siblings, 2 replies; 12+ messages in thread From: Laurent Vivier @ 2015-09-29 8:37 UTC (permalink / raw) To: David Gibson Cc: thuth, Michael S. Tsirkin, qemu-ppc, Alexander Graf, qemu-devel -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 On 29/09/2015 07:18, David Gibson wrote: > On Thu, Sep 24, 2015 at 12:27:39PM +0200, Laurent Vivier wrote: >> When DT node names for PCI devices are generated by SLOF, they >> are generated according to the type of the device (for instance, >> ethernet for virtio-net-pci device). >> >> Node name for hotplugged devices is generated by QEMU. This patch >> adds the mechanic to QEMU to create the node name according to >> the device type too. >> >> The data structure has been roughly copied from >> OpenBIOS/OpenHackware, node names from SLOF. >> >> Example: >> >> Hotplugging some PCI cards with QEMU monitor: >> >> device_add virtio-tablet-pci device_add virtio-serial-pci >> device_add virtio-mouse-pci device_add virtio-scsi-pci device_add >> virtio-gpu-pci device_add ne2k_pci device_add nec-usb-xhci >> device_add intel-hda >> >> What we can see in linux device tree: >> >> for dir in /proc/device-tree/pci@800000020000000/*@*/; do echo >> $dir cat $dir/name echo done >> >> WITHOUT this patch: >> >> /proc/device-tree/pci@800000020000000/pci@0/ pci >> /proc/device-tree/pci@800000020000000/pci@1/ pci >> /proc/device-tree/pci@800000020000000/pci@2/ pci >> /proc/device-tree/pci@800000020000000/pci@3/ pci >> /proc/device-tree/pci@800000020000000/pci@4/ pci >> /proc/device-tree/pci@800000020000000/pci@5/ pci >> /proc/device-tree/pci@800000020000000/pci@6/ pci >> /proc/device-tree/pci@800000020000000/pci@7/ pci >> >> WITH this patch: >> >> /proc/device-tree/pci@800000020000000/communication-controller@1/ >> >> communication-controller >> /proc/device-tree/pci@800000020000000/display@4/ display >> /proc/device-tree/pci@800000020000000/ethernet@5/ ethernet >> /proc/device-tree/pci@800000020000000/input-controller@0/ >> input-controller /proc/device-tree/pci@800000020000000/mouse@2/ >> mouse /proc/device-tree/pci@800000020000000/multimedia-device@7/ >> multimedia-device /proc/device-tree/pci@800000020000000/scsi@3/ >> scsi /proc/device-tree/pci@800000020000000/usb-xhci@6/ usb-xhci >> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com> Reviewed-by: >> Thomas Huth <thuth@redhat.com> > > Concept and approach seem good to me. > > >> --- hw/ppc/spapr_pci.c | 292 >> ++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file >> changed, 278 insertions(+), 14 deletions(-) >> >> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index >> a2feb4c..63eb28c 100644 --- a/hw/ppc/spapr_pci.c +++ >> b/hw/ppc/spapr_pci.c @@ -38,6 +38,7 @@ >> >> #include "hw/pci/pci_bridge.h" #include "hw/pci/pci_bus.h" >> +#include "hw/pci/pci_ids.h" #include "hw/ppc/spapr_drc.h" >> #include "sysemu/device_tree.h" >> >> @@ -944,6 +945,276 @@ static void >> populate_resource_props(PCIDevice *d, ResourceProps *rp) >> rp->assigned_len = assigned_idx * sizeof(ResourceFields); } >> >> +typedef struct PCIClass PCIClass; +typedef struct PCISubClass >> PCISubClass; +typedef struct PCIIFace PCIIFace; + +struct >> PCIIFace { + uint8_t iface; + const char *name; +}; + >> +struct PCISubClass { + uint8_t subclass; + const char >> *name; + const PCIIFace *iface; +}; +#define SUBCLASS(a) >> ((uint8_t)a) +#define IFACE(a) ((uint8_t)a) + +struct PCIClass >> { + const char *name; + const PCISubClass *subc; +}; + >> +static const PCISubClass undef_subclass[] = { + { >> IFACE(PCI_CLASS_NOT_DEFINED_VGA), "display", NULL }, > > These IFACE and SUBCLASS macro calls seem ugly to me. What's the > purpose of them? In fact, in case of subclass, the value is ((CLASS << 8) + SUBCLASS) and in case of iface is ((CLASS << 16) + (SUBCLASS << 8) + IFACE). To build the array we need either only the CLASS or the ICLASS of the value. These macros (which are indentical) take the low order byte to have only the subclass or the iface to put it in the array. And on this line, it's wrong, we should use SUBCLASS(): #define PCI_CLASS_NOT_DEFINED 0x0000 So class is "0x00" #define PCI_CLASS_NOT_DEFINED_VGA 0x0001 subclass is "0x01". For the case of iface: #define PCI_BASE_CLASS_SERIAL 0x0c class is "serial", 0x0c #define PCI_CLASS_SERIAL_USB 0x0c03 subclass is "usb", 0x03 #define PCI_CLASS_SERIAL_USB_XHCI 0x0c0330 iface is "xhci", 0x30 So of course I can remove macros SUBCLASS() and IFACE() and simply replace them by a cast "(uint8_t *)". > > [snip] >> +static const char *pci_find_device_name(uint8_t class, uint8_t >> subclass, + uint8_t >> iface) +{ + const PCIClass *pclass; + const PCISubClass >> *psubclass; + const PCIIFace *piface; + const char *name; >> + + if (class >= ARRAY_SIZE(pci_classes)) { + return >> "pci"; + } + + pclass = pci_classes + class; + name = >> pclass->name; + + if (pclass->subc == NULL) { + return >> name; + } + + psubclass = pclass->subc; + while >> (psubclass->subclass != 0xff) { + if (psubclass->subclass >> == subclass) { + name = psubclass->name; + >> break; + } + psubclass++; + } + + piface = >> psubclass->iface; + if (piface == NULL) { + return >> name; + } + while (piface->iface != 0xff) { + if >> (piface->iface == iface) { + name = piface->name; + >> break; + } + piface++; + } + + return name; >> +} + +static void pci_get_node_name(char *nodename, int len, >> PCIDevice *dev) +{ + int slot = PCI_SLOT(dev->devfn); + int >> func = PCI_FUNC(dev->devfn); + uint32_t ccode = >> pci_default_read_config(dev, PCI_CLASS_PROG, 3); + const char >> *name; + + name = pci_find_device_name((ccode >> 16) & 0xff, >> (ccode >> 8) & 0xff, + ccode & >> 0xff); + + if (func != 0) { + snprintf(nodename, len, >> "%s@%x,%x", name, slot, func); + } else { + >> snprintf(nodename, len, "%s@%x", name, slot); + } +} + static >> uint32_t spapr_phb_get_pci_drc_index(sPAPRPHBState *phb, >> PCIDevice *pdev); >> >> @@ -955,6 +1226,7 @@ static int >> spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int >> offset, int pci_status, err; char *buf = NULL; uint32_t drc_index >> = spapr_phb_get_pci_drc_index(sphb, dev); + uint32_t ccode = >> pci_default_read_config(dev, PCI_CLASS_PROG, 3); >> >> if (pci_default_read_config(dev, PCI_HEADER_TYPE, 1) == >> PCI_HEADER_TYPE_BRIDGE) { @@ -968,8 +1240,7 @@ static int >> spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int >> offset, pci_default_read_config(dev, PCI_DEVICE_ID, 2))); >> _FDT(fdt_setprop_cell(fdt, offset, "revision-id", >> pci_default_read_config(dev, PCI_REVISION_ID, 1))); - >> _FDT(fdt_setprop_cell(fdt, offset, "class-code", - >> pci_default_read_config(dev, PCI_CLASS_PROG, 3))); + >> _FDT(fdt_setprop_cell(fdt, offset, "class-code", ccode)); if >> (pci_default_read_config(dev, PCI_INTERRUPT_PIN, 1)) { >> _FDT(fdt_setprop_cell(fdt, offset, "interrupts", >> pci_default_read_config(dev, PCI_INTERRUPT_PIN, 1))); @@ -1010,11 >> +1281,10 @@ static int spapr_populate_pci_child_dt(PCIDevice >> *dev, void *fdt, int offset, _FDT(fdt_setprop(fdt, offset, >> "udf-supported", NULL, 0)); } >> >> - /* NOTE: this is normally generated by firmware via >> path/unit name, - * but in our case we must set it manually >> since it does not get - * processed by OF beforehand - >> */ - _FDT(fdt_setprop_string(fdt, offset, "name", "pci")); + >> _FDT(fdt_setprop_string(fdt, offset, "name", + >> pci_find_device_name((ccode >> 16) & 0xff, + >> (ccode >> 8) & 0xff, + >> ccode & 0xff))); > > Heh. This isn't wrong, but there's a non-obvious wrinkle as to > why. Generally flattened trees shouldn't require the 'name' > property (the consumer is supposed to infer that from the node > name). However, since this may also be used to generate tree > fragments which get passed through the PAPR dynamic reconfiguration > interface, and that *does* expect the name property to be passed > through. In fact, this is also the behavior of SLOF. > >> buf = spapr_phb_get_loc_code(sphb, dev); if (!buf) { >> error_report("Failed setting the ibm,loc-code"); @@ -1051,15 >> +1321,9 @@ static int spapr_create_pci_child_dt(sPAPRPHBState >> *phb, PCIDevice *dev, void *fdt, int node_offset) { int offset, >> ret; - int slot = PCI_SLOT(dev->devfn); - int func = >> PCI_FUNC(dev->devfn); char nodename[FDT_NAME_MAX]; >> >> - if (func != 0) { - snprintf(nodename, FDT_NAME_MAX, >> "pci@%x,%x", slot, func); - } else { - >> snprintf(nodename, FDT_NAME_MAX, "pci@%x", slot); - } + >> pci_get_node_name(nodename, FDT_NAME_MAX, dev); offset = >> fdt_add_subnode(fdt, node_offset, nodename); ret = >> spapr_populate_pci_child_dt(dev, fdt, offset, phb); >> > -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJWCk3LAAoJEPMMOL0/L748PKsP/2pFORLvqnU6oOB/RFQsAygW ux/EgoBE1eHS6XHus2hT73x5J8AGQMjmK7TtE1DRpEbNqWvcHAagsTvvgYj+swIY hydvsNj9tFvMNLsrZ7pX2xVBivjeydlKrEs0+cvO+RmBXV8O0npBlKvbRLDNZQl5 k//qBtKVNSzlej6GU3a9zSx/rR9dMqt2aQudw6xwbTx/olzC+tQU4Ddgz/iIQchj 8WyiZ+eyc744OEilRMMDa/jkS+1Bq4e3IP1V9BITtOkEtAlhmO2lilTCMVaSTBr8 Kdi5YsFY88g2nh3UkvHpc8Glrois7SaxpP7Z6pb2yj/KDuew3rf/Gjag5bkRTq6g NqgeBmnjyvJWqNM01I2jDD23c4luD/YChtGHF2lW8FET3Jca5D9/U4diXlITiFNd njrhK2SYq/2SfaUe2zeqRpHJE7kKOeUjhMo/n2SvEhA9qjnRwbfKZhAHAs+qZ2IH wDQ1SR19mWYpyN2CStOY6T+sCKYg4ZfuBnSkr/b5KQL/CjhXTV9ECoCrjEhvO7m2 5bpzX5gIgehSarHRLDdRrkUoZ+MHa116qjLa6x7pBhYGj7FY3L+ED1Gl5YJz7cQe J8MOqsr2uA1Yw5ZwwI+lwIJxI1/A/dRkD2nXDivu1/sFAVdVat80oJJ5jL4yIJ9E Piwez/HY+TsVtCsZCUDL =c7gD -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/2] spapr: generate DT node names 2015-09-29 8:37 ` Laurent Vivier @ 2015-09-29 9:40 ` Thomas Huth 2015-09-30 4:33 ` David Gibson 1 sibling, 0 replies; 12+ messages in thread From: Thomas Huth @ 2015-09-29 9:40 UTC (permalink / raw) To: Laurent Vivier, David Gibson Cc: Michael S. Tsirkin, qemu-ppc, Alexander Graf, qemu-devel [-- Attachment #1: Type: text/plain, Size: 1203 bytes --] On 29/09/15 10:37, Laurent Vivier wrote: > > > On 29/09/2015 07:18, David Gibson wrote: >> On Thu, Sep 24, 2015 at 12:27:39PM +0200, Laurent Vivier wrote: ... >>> - /* NOTE: this is normally generated by firmware via >>> path/unit name, - * but in our case we must set it manually >>> since it does not get - * processed by OF beforehand - >>> */ - _FDT(fdt_setprop_string(fdt, offset, "name", "pci")); + >>> _FDT(fdt_setprop_string(fdt, offset, "name", + >>> pci_find_device_name((ccode >> 16) & 0xff, + >>> (ccode >> 8) & 0xff, + >>> ccode & 0xff))); > >> Heh. This isn't wrong, but there's a non-obvious wrinkle as to >> why. Generally flattened trees shouldn't require the 'name' >> property (the consumer is supposed to infer that from the node >> name). However, since this may also be used to generate tree >> fragments which get passed through the PAPR dynamic reconfiguration >> interface, and that *does* expect the name property to be passed >> through. > > In fact, this is also the behavior of SLOF. Well, SLOF builds an Open Firmware device tree (not a flattened device tree), thus the "name" property is always required there. Thomas [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/2] spapr: generate DT node names 2015-09-29 8:37 ` Laurent Vivier 2015-09-29 9:40 ` Thomas Huth @ 2015-09-30 4:33 ` David Gibson 1 sibling, 0 replies; 12+ messages in thread From: David Gibson @ 2015-09-30 4:33 UTC (permalink / raw) To: Laurent Vivier Cc: thuth, Michael S. Tsirkin, qemu-ppc, Alexander Graf, qemu-devel [-- Attachment #1: Type: text/plain, Size: 5379 bytes --] On Tue, Sep 29, 2015 at 10:37:31AM +0200, Laurent Vivier wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA256 > > > > On 29/09/2015 07:18, David Gibson wrote: > > On Thu, Sep 24, 2015 at 12:27:39PM +0200, Laurent Vivier wrote: > >> When DT node names for PCI devices are generated by SLOF, they > >> are generated according to the type of the device (for instance, > >> ethernet for virtio-net-pci device). > >> > >> Node name for hotplugged devices is generated by QEMU. This patch > >> adds the mechanic to QEMU to create the node name according to > >> the device type too. > >> > >> The data structure has been roughly copied from > >> OpenBIOS/OpenHackware, node names from SLOF. > >> > >> Example: > >> > >> Hotplugging some PCI cards with QEMU monitor: > >> > >> device_add virtio-tablet-pci device_add virtio-serial-pci > >> device_add virtio-mouse-pci device_add virtio-scsi-pci device_add > >> virtio-gpu-pci device_add ne2k_pci device_add nec-usb-xhci > >> device_add intel-hda > >> > >> What we can see in linux device tree: > >> > >> for dir in /proc/device-tree/pci@800000020000000/*@*/; do echo > >> $dir cat $dir/name echo done > >> > >> WITHOUT this patch: > >> > >> /proc/device-tree/pci@800000020000000/pci@0/ pci > >> /proc/device-tree/pci@800000020000000/pci@1/ pci > >> /proc/device-tree/pci@800000020000000/pci@2/ pci > >> /proc/device-tree/pci@800000020000000/pci@3/ pci > >> /proc/device-tree/pci@800000020000000/pci@4/ pci > >> /proc/device-tree/pci@800000020000000/pci@5/ pci > >> /proc/device-tree/pci@800000020000000/pci@6/ pci > >> /proc/device-tree/pci@800000020000000/pci@7/ pci > >> > >> WITH this patch: > >> > >> /proc/device-tree/pci@800000020000000/communication-controller@1/ > >> > >> > communication-controller > >> /proc/device-tree/pci@800000020000000/display@4/ display > >> /proc/device-tree/pci@800000020000000/ethernet@5/ ethernet > >> /proc/device-tree/pci@800000020000000/input-controller@0/ > >> input-controller /proc/device-tree/pci@800000020000000/mouse@2/ > >> mouse /proc/device-tree/pci@800000020000000/multimedia-device@7/ > >> multimedia-device /proc/device-tree/pci@800000020000000/scsi@3/ > >> scsi /proc/device-tree/pci@800000020000000/usb-xhci@6/ usb-xhci > >> > >> Signed-off-by: Laurent Vivier <lvivier@redhat.com> Reviewed-by: > >> Thomas Huth <thuth@redhat.com> > > > > Concept and approach seem good to me. > > > > > >> --- hw/ppc/spapr_pci.c | 292 > >> ++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file > >> changed, 278 insertions(+), 14 deletions(-) > >> > >> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index > >> a2feb4c..63eb28c 100644 --- a/hw/ppc/spapr_pci.c +++ > >> b/hw/ppc/spapr_pci.c @@ -38,6 +38,7 @@ > >> > >> #include "hw/pci/pci_bridge.h" #include "hw/pci/pci_bus.h" > >> +#include "hw/pci/pci_ids.h" #include "hw/ppc/spapr_drc.h" > >> #include "sysemu/device_tree.h" > >> > >> @@ -944,6 +945,276 @@ static void > >> populate_resource_props(PCIDevice *d, ResourceProps *rp) > >> rp->assigned_len = assigned_idx * sizeof(ResourceFields); } > >> > >> +typedef struct PCIClass PCIClass; +typedef struct PCISubClass > >> PCISubClass; +typedef struct PCIIFace PCIIFace; + +struct > >> PCIIFace { + uint8_t iface; + const char *name; +}; + > >> +struct PCISubClass { + uint8_t subclass; + const char > >> *name; + const PCIIFace *iface; +}; +#define SUBCLASS(a) > >> ((uint8_t)a) +#define IFACE(a) ((uint8_t)a) + +struct PCIClass > >> { + const char *name; + const PCISubClass *subc; +}; + > >> +static const PCISubClass undef_subclass[] = { + { > >> IFACE(PCI_CLASS_NOT_DEFINED_VGA), "display", NULL }, > > > > These IFACE and SUBCLASS macro calls seem ugly to me. What's the > > purpose of them? > > In fact, in case of subclass, the value is ((CLASS << 8) + SUBCLASS) > and in case of iface is ((CLASS << 16) + (SUBCLASS << 8) + IFACE). > To build the array we need either only the CLASS or the ICLASS of the > value. These macros (which are indentical) take the low order byte to > have only the subclass or the iface to put it in the array. > > And on this line, it's wrong, we should use SUBCLASS(): > > #define PCI_CLASS_NOT_DEFINED 0x0000 > > So class is "0x00" > > #define PCI_CLASS_NOT_DEFINED_VGA 0x0001 > > subclass is "0x01". > > For the case of iface: > > #define PCI_BASE_CLASS_SERIAL 0x0c > > class is "serial", 0x0c > > #define PCI_CLASS_SERIAL_USB 0x0c03 > > subclass is "usb", 0x03 > > #define PCI_CLASS_SERIAL_USB_XHCI 0x0c0330 > > iface is "xhci", 0x30 > > So of course I can remove macros SUBCLASS() and IFACE() and simply > replace them by a cast "(uint8_t *)". Hm, ok. If the point of the macro is to mask out the relevant bits, I'd prefer to see an explicit (x & 0xff), rather than using the cast. The effect is the same, but it's more obvious what it's for. But.. I wonder if it might be simpler still to just store the whole value in the table, and only mask in the lookup function. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson [-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-09-30 4:40 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-09-24 10:27 [Qemu-devel] [PATCH v4 0/2] spapr: generate DT node names Laurent Vivier 2015-09-24 10:27 ` [Qemu-devel] [PATCH v4 1/2] PCI: add missing classes in pci_ids.h to build device tree Laurent Vivier 2015-09-24 10:45 ` Thomas Huth 2015-09-24 10:27 ` [Qemu-devel] [PATCH v4 2/2] spapr: generate DT node names Laurent Vivier 2015-09-24 23:29 ` Gavin Shan 2015-09-25 8:29 ` Laurent Vivier 2015-09-25 9:12 ` Michael S. Tsirkin 2015-09-25 10:21 ` Laurent Vivier 2015-09-29 5:18 ` David Gibson 2015-09-29 8:37 ` Laurent Vivier 2015-09-29 9:40 ` Thomas Huth 2015-09-30 4:33 ` David Gibson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).