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 <zuban32s@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
--- /dev/null
+++ b/src/fw/dev-pci.h
@@ -0,0 +1,50 @@
+#ifndef _PCI_CAP_H
+#define _PCI_CAP_H
+
+#include "types.h"
+
+/*
+
+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
+Data:
+
+u32 bus_res; minimum bus number to reserve;
+ this is necessary for PCI Express Root Ports
+ to support PCIE-to-PCI bridge hotplug
+u64 io; IO space to reserve
+u64 mem; non-prefetchable memory space to reserve
+u64 prefetchable_mem; prefetchable memory space to reserve
+
+If any field value in Data section is -1,
+it means that such kind of reservation
+is not needed and must be ignored.
+
+*/
+
+/* Offset of vendor-specific capability type field */
+#define PCI_CAP_REDHAT_TYPE 3
+
+/* List of valid Red Hat vendor-specific capability types */
+#define REDHAT_CAP_TYPE_QEMU 1
+
+
+/* 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 */
--
2.7.4
_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
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 <zuban32s@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
_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
2017-08-07 18:52 GMT+03:00 Marcel Apfelbaum <marcel@redhat.com>: > 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 <zuban32s@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, > Hi Marcel, >> --- /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. I meant cast to unsigned here (because we still use unsigned types), but if it can mislead someone I will change this. > >> +*/ >> + >> +/* 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. > This is actually an offset to this field >> + >> +/* 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. > What does the first 'RES' mean? >> + >> + >> +/* 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 -- Aleksandr Bezzubikov _______________________________________________ SeaBIOS mailing list SeaBIOS@seabios.org https://mail.coreboot.org/mailman/listinfo/seabios
On 07/08/2017 19:02, Alexander Bezzubikov wrote: > 2017-08-07 18:52 GMT+03:00 Marcel Apfelbaum <marcel@redhat.com>: >> 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 <zuban32s@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, >> > > Hi Marcel, > >>> --- /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. > > I meant cast to unsigned here (because we still use unsigned types), > but if it can mislead someone I will change this. > We should not document signed values for unsigned fields, even if the reason is "best practices." >> >>> +*/ >>> + >>> +/* 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. >> > > This is actually an offset to this field > OK, so it should end with 'offset' to be clear. I was mislead. >>> + >>> +/* 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. >> > > What does the first 'RES' mean? Resource. I don't mind you change it how you seem fit, just make it clear what it does. Is about resource reserving, not a general capability. Thanks, Marcel > >>> + >>> + >>> +/* 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 > > >
© 2016 - 2025 Red Hat, Inc.