Hi Akihiko,
On 10/2/24 11:24, Akihiko Odaki wrote:
vfio determines if rombar is explicitly enabled by inspecting QDict.
Inspecting QDict is not nice because QDict is untyped and depends on the
details on the external interface. Add an infrastructure to determine if
rombar is explicitly enabled to hw/pci. PCIDevice::rom_bar is changed to
have -1 by the default to tell rombar is explicitly enabled. It is
consistent with other properties like addr and romsize.
Signed-off-by: Akihiko Odaki <akihiko.od...@daynix.com>
---
include/hw/pci/pci_device.h | 5 +++++
hw/pci/pci.c | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
index d3dd0f64b273..54fa0676abf1 100644
--- a/include/hw/pci/pci_device.h
+++ b/include/hw/pci/pci_device.h
@@ -205,6 +205,11 @@ static inline uint16_t pci_get_bdf(PCIDevice *dev)
return PCI_BUILD_BDF(pci_bus_num(pci_get_bus(dev)), dev->devfn);
}
+static inline bool pci_rom_bar_explicitly_enabled(PCIDevice *dev)
+{
+ return dev->rom_bar && dev->rom_bar != -1;
Or return dev->rom_bar >= 0;
+}
+
uint16_t pci_requester_id(PCIDevice *dev);
/* DMA access functions */
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 76080af580d7..d08548d8ffe9 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -71,7 +71,7 @@ static Property pci_props[] = {
DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
DEFINE_PROP_STRING("romfile", PCIDevice, romfile),
DEFINE_PROP_UINT32("romsize", PCIDevice, romsize, -1),
- DEFINE_PROP_UINT32("rombar", PCIDevice, rom_bar, 1),
+ DEFINE_PROP_UINT32("rombar", PCIDevice, rom_bar, -1),
You are changing a default affecting many devices. Please
document in the commit description it is safe because there
are few uses and you audited them:
$ git grep -w rom_bar
hw/display/qxl.c:328: QXLRom *rom =
memory_region_get_ram_ptr(&d->rom_bar);
hw/display/qxl.c:431: qxl_set_dirty(&qxl->rom_bar, 0, qxl->rom_size);
hw/display/qxl.c:1048: QXLRom *rom =
memory_region_get_ram_ptr(&qxl->rom_bar);
hw/display/qxl.c:2131: memory_region_init_rom(&qxl->rom_bar,
OBJECT(qxl), "qxl.vrom",
hw/display/qxl.c:2154:
PCI_BASE_ADDRESS_SPACE_MEMORY, &qxl->rom_bar);
hw/display/qxl.h:101: MemoryRegion rom_bar;
hw/pci/pci.c:74: DEFINE_PROP_UINT32("rombar", PCIDevice, rom_bar, 1),
hw/pci/pci.c:2329: if (!pdev->rom_bar) {
hw/vfio/pci.c:1019: if (vdev->pdev.romfile || !vdev->pdev.rom_bar) {
hw/xen/xen_pt_load_rom.c:29: if (dev->romfile || !dev->rom_bar) {
include/hw/pci/pci_device.h:150: uint32_t rom_bar;
Alternatively split this patch in 2, first introduce
pci_rom_bar_explicitly_enabled() and use it, then change the default
value and adapt it.
Regards,
Phil.
