[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] spapr: Adjust firmware path of PCI devices
|
From: |
Alexey Kardashevskiy |
|
Subject: |
Re: [PATCH] spapr: Adjust firmware path of PCI devices |
|
Date: |
Wed, 27 Jan 2021 12:28:57 +1100 |
|
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:85.0) Gecko/20100101 Thunderbird/85.0 |
On 25/01/2021 21:23, Greg Kurz wrote:
On Sat, 23 Jan 2021 13:36:34 +1100
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
On 23/01/2021 04:01, Greg Kurz wrote:
It is currently not possible to perform a strict boot from USB storage:
$ qemu-system-ppc64 -accel kvm -nodefaults -nographic -serial stdio \
-boot strict=on \
-device qemu-xhci \
-device usb-storage,drive=disk,bootindex=0 \
-blockdev driver=file,node-name=disk,filename=fedora-ppc64le.qcow2
SLOF **********************************************************************
QEMU Starting
Build Date = Jul 17 2020 11:15:24
FW Version = git-e18ddad8516ff2cf
Press "s" to enter Open Firmware.
Populating /vdevice methods
Populating /vdevice/vty@71000000
Populating /vdevice/nvram@71000001
Populating /pci@800000020000000
00 0000 (D) : 1b36 000d serial bus [ usb-xhci ]
No NVRAM common partition, re-initializing...
Scanning USB
XHCI: Initializing
USB Storage
SCSI: Looking for devices
101000000000000 DISK : "QEMU QEMU HARDDISK 2.5+"
Using default console: /vdevice/vty@71000000
Welcome to Open Firmware
Copyright (c) 2004, 2017 IBM Corporation All rights reserved.
This program and the accompanying materials are made available
under the terms of the BSD License available at
http://www.opensource.org/licenses/bsd-license.php
Trying to load: from:
/pci@800000020000000/usb@0/storage@1/disk@101000000000000 ...
E3405: No such device
E3407: Load failed
Type 'boot' and press return to continue booting the system.
Type 'reset-all' and press return to reboot the system.
Ready!
0 >
The device tree handed over by QEMU to SLOF indeed contains:
qemu,boot-list =
"/pci@800000020000000/usb@0/storage@1/disk@101000000000000 HALT";
but the device node is named usb-xhci@0, not usb@0.
I'd expect it to be a return of qdev_fw_name() so in this case something
like "nec-usb-xhci" (which would still be broken) but seeing a plain
"usb" is a bit strange.
The logic under get_boot_devices_list() is a bit hard to follow
because of the multiple indirections, but AFAICT it doesn't seem
to rely on qdev_fw_name() to get the names.
None of the XHCI devices seem to be setting DeviceClass::fw_name anyway:
$ git grep fw_name hw/usb/
hw/usb/bus.c: qdev_fw_name(qdev), nr);
hw/usb/dev-hub.c: dc->fw_name = "hub";
hw/usb/dev-mtp.c: dc->fw_name = "mtp";
hw/usb/dev-network.c: dc->fw_name = "network";
hw/usb/dev-storage.c: dc->fw_name = "storage";
hw/usb/dev-uas.c: dc->fw_name = "storage";
The plain "usb" naming comes from PCI, which has its own naming
logic for PCI devices (which qemu-xhci happens to be) :
Right, this was the confusing bit for me. I thought that by just setting
dc->fw_name to what we put in the DT should be enough but it is not.
#0 0x0000000100319474 in pci_dev_fw_name (len=33, buf=0x7fffffffd4c8 "\020",
dev=0x7fffc3320010) at ../../hw/pci/pci.c:2533
#1 0x0000000100319474 in pcibus_get_fw_dev_path (dev=0x7fffc3320010) at
../../hw/pci/pci.c:2550
#2 0x000000010053118c in bus_get_fw_dev_path (dev=0x7fffc3320010, bus=<optimized
out>) at ../../hw/core/qdev-fw.c:38
#3 0x000000010053118c in qdev_get_fw_dev_path_helper (dev=0x7fffc3320010,
p=0x7fffffffd728 "/pci@800000020000000/", size=128) at
../../hw/core/qdev-fw.c:72
#4 0x0000000100531064 in qdev_get_fw_dev_path_helper (dev=0x101c864a0, p=0x7fffffffd728
"/pci@800000020000000/", size=128) at ../../hw/core/qdev-fw.c:69
#5 0x0000000100531064 in qdev_get_fw_dev_path_helper (dev=0x1019f3560, p=0x7fffffffd728
"/pci@800000020000000/", size=128) at ../../hw/core/qdev-fw.c:69
#6 0x00000001005312f0 in qdev_get_fw_dev_path (dev=<optimized out>) at
../../hw/core/qdev-fw.c:91
#7 0x0000000100588a68 in get_boot_device_path (dev=<optimized out>,
ignore_suffixes=<optimized out>, ignore_suffixes@entry=true, suffix=<optimized out>)
at ../../softmmu/bootdevice.c:211
#8 0x0000000100589540 in get_boot_devices_list (size=0x7fffffffd990) at
../../softmmu/bootdevice.c:257
#9 0x0000000100606764 in spapr_dt_chosen (reset=true, fdt=0x7fffc26f0010,
spapr=0x10149aef0) at ../../hw/ppc/spapr.c:1019
While your patch works, I wonder if we should assign fw_name to all pci
nodes to avoid similar problems in the future? Thanks,
Not sure to understand "assign fw_name to all pci nodes" ...
Basically this:
=====
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index de0fae10ab9c..8a286419aaf8 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2508,7 +2508,12 @@ static char *pci_dev_fw_name(DeviceState *dev,
char *buf, int len)
const char *name = NULL;
const pci_class_desc *desc = pci_class_descriptions;
int class = pci_get_word(d->config + PCI_CLASS_DEVICE);
+ DeviceClass *dc = DEVICE_GET_CLASS(dev);
+ if (dc->fw_name) {
+ pstrcpy(buf, len, dc->fw_name);
+ return buf;
+ }
while (desc->desc &&
(class & ~desc->fw_ign_bits) !=
(desc->class & ~desc->fw_ign_bits)) {
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 0a418f1e6711..057dd384ada7 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1467,8 +1467,14 @@ int spapr_pci_dt_populate(SpaprDrc *drc,
SpaprMachineState *spapr,
HotplugHandler *plug_handler = qdev_get_hotplug_handler(drc->dev);
SpaprPhbState *sphb = SPAPR_PCI_HOST_BRIDGE(plug_handler);
PCIDevice *pdev = PCI_DEVICE(drc->dev);
+ DeviceState *dev = DEVICE(pdev);
+ uint32_t ccode = pci_default_read_config(pdev, PCI_CLASS_PROG, 3);
+ DeviceClass *dc = DEVICE_GET_CLASS(dev);
*fdt_start_offset = spapr_dt_pci_device(sphb, pdev, fdt, 0);
+
+ dc->fw_name = g_strdup(dt_name_from_class((ccode >> 16) & 0xff,
(ccode >> 8) & 0xff,
+ ccode & 0xff));
return 0;
}
=====
Note this is just to demonstrate the idea (this works though) :)
(changing the device class is way too late, would need to dig QOM a bit,
also need to do the same for hotplugged devices).
But the point is that pci_dev_fw_name() (has "_fw_name"!) should not
ignore dc->fw_name. Thanks,
--
Alexey