[v2] Allow RedHat PCI bridges reserve more buses than necessary during init

[SeaBIOS] [RFC PATCH v2 3/4] pci: add QEMU-specific PCI capability structure

Posted by Aleksandr Bezzubikov 7 years, 9 months ago

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.

Sizes of limits match ones from 
PCI Type 1 Configuration Space Header,
number of buses to reserve occupies only 1 byte 
since it is the size of Subordinate Bus Number register.

Signed-off-by: Aleksandr Bezzubikov <zuban32s@gmail.com>
---
 src/hw/pci_cap.h | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100644 src/hw/pci_cap.h

diff --git a/src/hw/pci_cap.h b/src/hw/pci_cap.h
new file mode 100644
index 0000000..1382b0b
--- /dev/null
+++ b/src/hw/pci_cap.h
@@ -0,0 +1,23 @@
+#ifndef _PCI_CAP_H
+#define _PCI_CAP_H
+
+#include "types.h"
+
+struct vendor_pci_cap {
+    u8 id;
+    u8 next;
+    u8 len;
+};
+
+struct redhat_pci_bridge_cap {
+    struct vendor_pci_cap hdr;
+    u8 bus_res;
+    u32 pref_lim_upper;
+    u16 pref_lim;
+    u16 mem_lim;
+    u16 io_lim_upper;
+    u8 io_lim;
+    u8 padd;
+};
+
+#endif /* _PCI_CAP_H */
-- 
2.7.4


_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios

Re: [Qemu-devel] [RFC PATCH v2 3/4] pci: add QEMU-specific PCI capability structure

Posted by Michael S. Tsirkin 7 years, 9 months ago

On Sun, Jul 23, 2017 at 01:11:49AM +0300, 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.
> 
> Sizes of limits match ones from 
> PCI Type 1 Configuration Space Header,
> number of buses to reserve occupies only 1 byte 
> since it is the size of Subordinate Bus Number register.
> 
> Signed-off-by: Aleksandr Bezzubikov <zuban32s@gmail.com>
> ---
>  src/hw/pci_cap.h | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>  create mode 100644 src/hw/pci_cap.h
> 
> diff --git a/src/hw/pci_cap.h b/src/hw/pci_cap.h
> new file mode 100644
> index 0000000..1382b0b
> --- /dev/null
> +++ b/src/hw/pci_cap.h
> @@ -0,0 +1,23 @@
> +#ifndef _PCI_CAP_H
> +#define _PCI_CAP_H
> +
> +#include "types.h"
> +
> +struct vendor_pci_cap {
> +    u8 id;
> +    u8 next;
> +    u8 len;
> +};
> +
> +struct redhat_pci_bridge_cap {
> +    struct vendor_pci_cap hdr;

You want to add some kind of identifier here after
the header, such that more capabilities can be added
in future without breaking this one.

> +    u8 bus_res;
> +    u32 pref_lim_upper;
> +    u16 pref_lim;
> +    u16 mem_lim;
> +    u16 io_lim_upper;
> +    u8 io_lim;
> +    u8 padd;

Please add documentation.


> +};
> +
> +#endif /* _PCI_CAP_H */
> -- 
> 2.7.4

Re: [Qemu-devel] [RFC PATCH v2 3/4] pci: add QEMU-specific PCI capability structure

Posted by Alexander Bezzubikov 7 years, 9 months ago

2017-07-23 5:44 GMT+03:00 Michael S. Tsirkin <mst@redhat.com>:

> On Sun, Jul 23, 2017 at 01:11:49AM +0300, 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.
> >
> > Sizes of limits match ones from
> > PCI Type 1 Configuration Space Header,
> > number of buses to reserve occupies only 1 byte
> > since it is the size of Subordinate Bus Number register.
> >
> > Signed-off-by: Aleksandr Bezzubikov <zuban32s@gmail.com>
> > ---
> >  src/hw/pci_cap.h | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> >  create mode 100644 src/hw/pci_cap.h
> >
> > diff --git a/src/hw/pci_cap.h b/src/hw/pci_cap.h
> > new file mode 100644
> > index 0000000..1382b0b
> > --- /dev/null
> > +++ b/src/hw/pci_cap.h
> > @@ -0,0 +1,23 @@
> > +#ifndef _PCI_CAP_H
> > +#define _PCI_CAP_H
> > +
> > +#include "types.h"
> > +
> > +struct vendor_pci_cap {
> > +    u8 id;
> > +    u8 next;
> > +    u8 len;
> > +};
> > +
> > +struct redhat_pci_bridge_cap {
> > +    struct vendor_pci_cap hdr;
>
> Hi Michael,
Thanks for the quick reply.


> You want to add some kind of identifier here after
> the header, such that more capabilities can be added
> in future without breaking this one.
>

You mean to distinguish different vendor-specific capabilities?
Agreed if so, will add it in the next version.


>
> > +    u8 bus_res;
> > +    u32 pref_lim_upper;
> > +    u16 pref_lim;
> > +    u16 mem_lim;
> > +    u16 io_lim_upper;
> > +    u8 io_lim;
> > +    u8 padd;
>
> Please add documentation.
>
>
> > +};
> > +
> > +#endif /* _PCI_CAP_H */
> > --
> > 2.7.4
>



-- 
Alexander Bezzubikov

Re: [SeaBIOS] [RFC PATCH v2 3/4] pci: add QEMU-specific PCI capability structure

Posted by Kevin O'Connor 7 years, 9 months ago

On Sun, Jul 23, 2017 at 01:11:49AM +0300, 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.
> 
> Sizes of limits match ones from 
> PCI Type 1 Configuration Space Header,
> number of buses to reserve occupies only 1 byte 
> since it is the size of Subordinate Bus Number register.
> 
> Signed-off-by: Aleksandr Bezzubikov <zuban32s@gmail.com>
> ---
>  src/hw/pci_cap.h | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>  create mode 100644 src/hw/pci_cap.h
> 
> diff --git a/src/hw/pci_cap.h b/src/hw/pci_cap.h
> new file mode 100644
> index 0000000..1382b0b
> --- /dev/null
> +++ b/src/hw/pci_cap.h
> @@ -0,0 +1,23 @@
> +#ifndef _PCI_CAP_H
> +#define _PCI_CAP_H
> +
> +#include "types.h"
> +
> +struct vendor_pci_cap {
> +    u8 id;
> +    u8 next;
> +    u8 len;
> +};

Thanks.  If you respin this series, please add this header to the
src/fw/ directory instead of src/hw/.  Also, I'd prefer to avoid a
"pci_" prefix on the header as it makes it seem similar to the
existing pci_regs.h and pci_ids.h headers which are a bit different -
how about src/fw/dev-pci.h instead?

-Kevin

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [RFC PATCH v2 3/4] pci: add QEMU-specific PCI capability structure

Posted by Alexander Bezzubikov 7 years, 9 months ago

2017-07-23 19:30 GMT+03:00 Kevin O'Connor <kevin@koconnor.net>:

> On Sun, Jul 23, 2017 at 01:11:49AM +0300, 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.
> >
> > Sizes of limits match ones from
> > PCI Type 1 Configuration Space Header,
> > number of buses to reserve occupies only 1 byte
> > since it is the size of Subordinate Bus Number register.
> >
> > Signed-off-by: Aleksandr Bezzubikov <zuban32s@gmail.com>
> > ---
> >  src/hw/pci_cap.h | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> >  create mode 100644 src/hw/pci_cap.h
> >
> > diff --git a/src/hw/pci_cap.h b/src/hw/pci_cap.h
> > new file mode 100644
> > index 0000000..1382b0b
> > --- /dev/null
> > +++ b/src/hw/pci_cap.h
> > @@ -0,0 +1,23 @@
> > +#ifndef _PCI_CAP_H
> > +#define _PCI_CAP_H
> > +
> > +#include "types.h"
> > +
> > +struct vendor_pci_cap {
> > +    u8 id;
> > +    u8 next;
> > +    u8 len;
> > +};
>
> Thanks.

Thanks for the review.


> If you respin this series, please add this header to the
> src/fw/ directory instead of src/hw/.  Also, I'd prefer to avoid a
> "pci_" prefix on the header as it makes it seem similar to the
> existing pci_regs.h and pci_ids.h headers which are a bit different -
> how about src/fw/dev-pci.h instead?
>
> -Kevin
>

No objections, Will do that in v3.
Except 'pci_' prefix - it's still a PCI capability, isn't it?


-- 
Alexander Bezzubikov
_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios