On 05/08/2017 23:29, Aleksandr Bezzubikov wrote:
On PCI init PCI bridge devices may need some
extra info about bus number to reserve, IO, memory and
prefetchable memory limits. QEMU can provide this
with special vendor-specific PCI capability.
This capability is intended to be used only
for Red Hat PCI bridges, i.e. QEMU cooperation.
Signed-off-by: Aleksandr Bezzubikov <zuban...@gmail.com>
---
src/fw/dev-pci.h | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
create mode 100644 src/fw/dev-pci.h
diff --git a/src/fw/dev-pci.h b/src/fw/dev-pci.h
new file mode 100644
index 0000000..2c8ddb0
Hi Aleksandr,
--- /dev/null
+++ b/src/fw/dev-pci.h
@@ -0,0 +1,50 @@
+#ifndef _PCI_CAP_H
+#define _PCI_CAP_H
+
+#include "types.h"
+
+/*
+
Please use the standard comment:
/*
*
*
*/
+QEMU-specific vendor(Red Hat)-specific capability.
+It's intended to provide some hints for firmware to init PCI devices.
+
+Its structure is shown below:
+
+Header:
+
+u8 id; Standard PCI Capability Header field
+u8 next; Standard PCI Capability Header field
+u8 len; Standard PCI Capability Header field
+u8 type; Red Hat vendor-specific capability type:
+ now only REDHAT_CAP_TYP_QEMU=1 exists
Typo o the line before, but I think you don't need it there.
+Data:
+
+u32 bus_res; minimum bus number to reserve;
+ this is necessary for PCI Express Root Ports
+ to support PCIE-to-PCI bridge hotplug
I would add a broader class of usage:
necessary for nesting PCI bridges hotplug.
+u64 io; IO space to reserve
+u64 mem; non-prefetchable memory space to reserve
+u64 prefetchable_mem; prefetchable memory space to reserve
+
Layout looks good to me.
+If any field value in Data section is -1,
+it means that such kind of reservation
+is not needed and must be ignored.
+
-1 is not a valid value for unsigned fields, you may
want to say 0xff..f or some other way.
+*/
+
+/* Offset of vendor-specific capability type field */
+#define PCI_CAP_REDHAT_TYPE 3
May I ask why why '3'? I am not against it, I just
want to understand the number.
+
+/* List of valid Red Hat vendor-specific capability types */
+#define REDHAT_CAP_TYPE_QEMU 1
I think I pointed this in another thread, the name is
too vague, please change it to something like:
REDHAT_CAP_RES_RESERVE_QEMU
that narrows down the intend.
+
+
+/* Offsets of QEMU capability fields */
+#define QEMU_PCI_CAP_BUS_RES 4
+#define QEMU_PCI_CAP_LIMITS_OFFSET 8
+#define QEMU_PCI_CAP_IO 8
+#define QEMU_PCI_CAP_MEM 16
+#define QEMU_PCI_CAP_PREF_MEM 24
+#define QEMU_PCI_CAP_SIZE 32
+
+#endif /* _PCI_CAP_H */
The layout looks good to me.
Thanks,
Marcel
