Re: [SeaBIOS] [PATCH v3 3/3] pci: recognize RH PCI legacy bridge resource reservation capability
Hi Marcel,
On 8/25/2018 11:59 PM, Marcel Apfelbaum wrote:
On 08/24/2018 11:53 AM, Jing Liu wrote:
Enable the firmware recognizing RedHat legacy PCI bridge device ID,
so QEMU can reserve additional PCI bridge resource capability.
Change the debug level lower to 3 when it is non-QEMU bridge.
Signed-off-by: Jing Liu
---
src/fw/pciinit.c | 50
+-
src/hw/pci_ids.h | 1 +
2 files changed, 30 insertions(+), 21 deletions(-)
diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
index 62a32f1..c0634bc 100644
--- a/src/fw/pciinit.c
+++ b/src/fw/pciinit.c
@@ -525,30 +525,38 @@ static void pci_bios_init_platform(void)
static u8 pci_find_resource_reserve_capability(u16 bdf)
{
- if (pci_config_readw(bdf, PCI_VENDOR_ID) == PCI_VENDOR_ID_REDHAT &&
- pci_config_readw(bdf, PCI_DEVICE_ID) ==
- PCI_DEVICE_ID_REDHAT_ROOT_PORT) {
- u8 cap = 0;
- do {
- cap = pci_find_capability(bdf, PCI_CAP_ID_VNDR, cap);
- } while (cap &&
- pci_config_readb(bdf, cap +
PCI_CAP_REDHAT_TYPE_OFFSET) !=
- REDHAT_CAP_RESOURCE_RESERVE);
- if (cap) {
- u8 cap_len = pci_config_readb(bdf, cap + PCI_CAP_FLAGS);
- if (cap_len < RES_RESERVE_CAP_SIZE) {
- dprintf(1, "PCI: QEMU resource reserve cap length %d
is invalid\n",
- cap_len);
- return 0;
- }
- } else {
- dprintf(1, "PCI: QEMU resource reserve cap not found\n");
+ u16 device_id;
+
+ if (pci_config_readw(bdf, PCI_VENDOR_ID) != PCI_VENDOR_ID_REDHAT) {
+ dprintf(3, "PCI: This is non-QEMU bridge.\n");
+ return 0;
+ }
+
+ device_id = pci_config_readw(bdf, PCI_DEVICE_ID);
+
+ if (device_id != PCI_DEVICE_ID_REDHAT_ROOT_PORT &&
+ device_id != PCI_DEVICE_ID_REDHAT_BRIDGE) {
+ dprintf(1, "PCI: QEMU resource reserve cap device ID doesn't
match.\n");
+ return 0;
+ }
+ u8 cap = 0;
+
+ do {
+ cap = pci_find_capability(bdf, PCI_CAP_ID_VNDR, cap);
+ } while (cap &&
+ pci_config_readb(bdf, cap + PCI_CAP_REDHAT_TYPE_OFFSET) !=
+ REDHAT_CAP_RESOURCE_RESERVE);
+ if (cap) {
+ u8 cap_len = pci_config_readb(bdf, cap + PCI_CAP_FLAGS);
+ if (cap_len < RES_RESERVE_CAP_SIZE) {
+ dprintf(1, "PCI: QEMU resource reserve cap length %d is
invalid\n",
+ cap_len);
+ return 0;
}
- return cap;
} else {
- dprintf(1, "PCI: QEMU resource reserve cap VID or DID doesn't
match.\n");
- return 0;
I am sorry for the late review.
Did you drop the above line in purpose?
Thanks for the review!
I replaced the above report to following phase.
Check the vendor-id and device-id respectively.
+if (pci_config_readw(bdf, PCI_VENDOR_ID) != PCI_VENDOR_ID_REDHAT) {
+dprintf(3, "PCI: This is non-QEMU bridge.\n");
+return 0;
+}
+
+device_id = pci_config_readw(bdf, PCI_DEVICE_ID);
+
+if (device_id != PCI_DEVICE_ID_REDHAT_ROOT_PORT &&
+device_id != PCI_DEVICE_ID_REDHAT_BRIDGE) {
+dprintf(1, "PCI: QEMU resource reserve cap device ID doesn't
match.\n");
+return 0;
+}
Thanks,
Jing
Thanks,
Marcel
___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
Re: [SeaBIOS] [RFC v2 1/3] fw/pciinit: Recognize pxb-pcie-dev device
Marcel Apfelbaum 于2018年8月25日周六 下午4:11写道：
>
>
>
> On 08/09/2018 08:43 AM, Zihan Yang wrote:
> > QEMU q35 uses pxb-pcie-dev to enable multiple host bridges, this patch
> > recognizes such devices in seabios and add corresponding e820 entry.
> >
> > MCFG base and size are already setup in QEMU, we just need to read it
> >
> > Signed-off-by: Zihan Yang
> > ---
> > src/fw/paravirt.c | 1 -
> > src/fw/pciinit.c | 17 +
> > src/hw/pci_ids.h | 1 +
> > 3 files changed, 18 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
> > index 0770c47..6b14542 100644
> > --- a/src/fw/paravirt.c
> > +++ b/src/fw/paravirt.c
> > @@ -197,7 +197,6 @@ qemu_platform_setup(void)
> > if (!loader_err)
> > warn_internalerror();
> > }
> > -
>
> No need for this chunk.
>
> > acpi_setup();
> > }
> >
> > diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
> > index 3a2f747..6e6a434 100644
> > --- a/src/fw/pciinit.c
> > +++ b/src/fw/pciinit.c
> > @@ -507,11 +507,28 @@ static void mch_mem_addr_setup(struct pci_device
> > *dev, void *arg)
> > pci_io_low_end = acpi_pm_base;
> > }
> >
> > +static void pxb_mem_addr_setup(struct pci_device *dev, void *arg)
> > +{
> > +union u64_u32_u mcfg_base;
> > +mcfg_base.lo = pci_config_readl(dev->bdf, Q35_HOST_BRIDGE_PCIEXBAR);
> > +mcfg_base.hi = pci_config_readl(dev->bdf, Q35_HOST_BRIDGE_PCIEXBAR +
> > 4);
> > +
> > +// Fix me! Use another meaningful macro
> > +u32 mcfg_size = pci_config_readl(dev->bdf, Q35_HOST_BRIDGE_PCIEXBAR +
> > 8);
>
> That is indeed strange. If I got it right, this is the address of Q35
> host bridge,
> we want other addresses here.
I will try to find another address in next version.
> > +
> > +/* Skip config write here as the qemu-level objects are already setup,
> > we
> > + * read mcfg_base and mcfg_size from it just now. Instead, we directly
> > add
> > + * this item to e820 */
> > +e820_add(mcfg_base.val, mcfg_size, E820_RESERVED);
>
> So did QEMU configure the address or only the size?
QEMU configure both the address and the size. Since the base address
is right behind
the ram_over_4g and the size depends on the number of buses we want to use, they
are not fixed anymore. So I just let qemu configures them both and
seabios only reads
them out. Maybe after we resolve the design issue, we can 'truly' let
seabios to fully
decide the base address and size.
> > +}
> > +
> > static const struct pci_device_id pci_platform_tbl[] = {
> > PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82441,
> > i440fx_mem_addr_setup),
> > PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_Q35_MCH,
> > mch_mem_addr_setup),
> > +PCI_DEVICE(PCI_VENDOR_ID_REDHAT, PCI_DEVICE_ID_REDHAT_PXB_HOST,
> > + ),
>
> Will this recognize only the pxb-pcie bridge, or also the legacy pxb one?
> Because we plan to support only the pxb-pcie version.
The value is 0x000b, which is pxb-pcie in qemu. The corresponding macro is
PCI_DEVICE_ID_REDHAT_PXB_PCIE. The value for PXB is 0x0009, and is
defined with another macro PCI_DEVICE_ID_REDHAT_PXB.
> Thanks,
> Marcel
>
> > PCI_DEVICE_END
> > };
> >
> > diff --git a/src/hw/pci_ids.h b/src/hw/pci_ids.h
> > index 38fa2ca..35096ea 100644
> > --- a/src/hw/pci_ids.h
> > +++ b/src/hw/pci_ids.h
> > @@ -2265,6 +2265,7 @@
> >
> > #define PCI_VENDOR_ID_REDHAT0x1b36
> > #define PCI_DEVICE_ID_REDHAT_ROOT_PORT 0x000C
> > +#define PCI_DEVICE_ID_REDHAT_PXB_HOST0x000B
> >
> > #define PCI_VENDOR_ID_TEKRAM0x1de1
> > #define PCI_DEVICE_ID_TEKRAM_DC290 0xdc29
>
___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
Re: [SeaBIOS] [RFC v2 2/3] pci_device: Add pci domain support
Marcel Apfelbaum 于2018年8月25日周六 下午7:53写道：
>
>
>
> On 08/09/2018 08:43 AM, Zihan Yang wrote:
> > Most part of seabios assume only PCI domain 0. This patch adds support
> > for multiple domain in pci devices, which involves some API changes.
> >
> > For compatibility, interfaces such as pci_config_read[b|w|l] still exist
> > so that existing domain 0 devices needs no modification, but whenever a
> > device wants to reside in different domain, it should add *_dom suffix to
> > above functions, e.g, pci_config_readl_dom(..., domain_nr) to read from
> > specific host bridge other than q35 host bridge.
>
> It is not related only to q35. Is about PCI hosts bridges others
> that the main one.
I see, I will correct it.
> > Also, the user should
> > check the device domain when using foreachpci() macro to fileter undesired
> > devices that reside in a different domain.
> >
> > Signed-off-by: Zihan Yang
> > ---
> > src/fw/coreboot.c | 2 +-
> > src/fw/csm.c | 2 +-
> > src/fw/paravirt.c | 2 +-
> > src/fw/pciinit.c | 261
> > ++---
> > src/hw/pci.c | 69 +++---
> > src/hw/pci.h | 42 ++---
> > src/hw/pci_ids.h | 7 +-
> > src/hw/pcidevice.c | 8 +-
> > src/hw/pcidevice.h | 4 +-
> > 9 files changed, 227 insertions(+), 170 deletions(-)
> >
> > diff --git a/src/fw/coreboot.c b/src/fw/coreboot.c
> > index 7c0954b..c955dfd 100644
> > --- a/src/fw/coreboot.c
> > +++ b/src/fw/coreboot.c
> > @@ -254,7 +254,7 @@ coreboot_platform_setup(void)
> > {
> > if (!CONFIG_COREBOOT)
> > return;
> > -pci_probe_devices();
> > +pci_probe_devices(0);
> >
> > struct cb_memory *cbm = CBMemTable;
> > if (!cbm)
> > diff --git a/src/fw/csm.c b/src/fw/csm.c
> > index 03b4bb8..e94f614 100644
> > --- a/src/fw/csm.c
> > +++ b/src/fw/csm.c
> > @@ -63,7 +63,7 @@ static void
> > csm_maininit(struct bregs *regs)
> > {
> > interface_init();
> > -pci_probe_devices();
> > +pci_probe_devices(0);
> >
> > csm_compat_table.PnPInstallationCheckSegment = SEG_BIOS;
> > csm_compat_table.PnPInstallationCheckOffset = get_pnp_offset();
> > diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
> > index 6b14542..ef4d487 100644
> > --- a/src/fw/paravirt.c
> > +++ b/src/fw/paravirt.c
> > @@ -155,7 +155,7 @@ qemu_platform_setup(void)
> > return;
> >
> > if (runningOnXen()) {
> > -pci_probe_devices();
> > +pci_probe_devices(0);
> > xen_hypercall_setup();
> > xen_biostable_setup();
> > return;
> > diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
> > index 6e6a434..fcdcd38 100644
> > --- a/src/fw/pciinit.c
> > +++ b/src/fw/pciinit.c
> > @@ -51,6 +51,7 @@ u64 pcimem_end = BUILD_PCIMEM_END;
> > u64 pcimem64_start = BUILD_PCIMEM64_START;
> > u64 pcimem64_end = BUILD_PCIMEM64_END;
> > u64 pci_io_low_end = 0xa000;
> > +u64 pxb_mcfg_size = 0;
> >
> > struct pci_region_entry {
> > struct pci_device *dev;
> > @@ -88,9 +89,9 @@ static void
> > pci_set_io_region_addr(struct pci_device *pci, int bar, u64 addr, int
> > is64)
> > {
> > u32 ofs = pci_bar(pci, bar);
> > -pci_config_writel(pci->bdf, ofs, addr);
> > +pci_config_writel_dom(pci->bdf, ofs, addr, pci->domain_nr);
> > if (is64)
> > -pci_config_writel(pci->bdf, ofs + 4, addr >> 32);
> > +pci_config_writel_dom(pci->bdf, ofs + 4, addr >> 32,
> > pci->domain_nr);
> > }
> >
> >
> > @@ -405,25 +406,29 @@ static void pci_bios_init_device(struct pci_device
> > *pci)
> >
> > /* map the interrupt */
> > u16 bdf = pci->bdf;
> > -int pin = pci_config_readb(bdf, PCI_INTERRUPT_PIN);
> > +int pin = pci_config_readb_dom(bdf, PCI_INTERRUPT_PIN, pci->domain_nr);
> > if (pin != 0)
> > -pci_config_writeb(bdf, PCI_INTERRUPT_LINE, pci_slot_get_irq(pci,
> > pin));
> > +pci_config_writeb_dom(bdf, PCI_INTERRUPT_LINE,
> > pci_slot_get_irq(pci, pin),
> > + pci->domain_nr);
> >
> > pci_init_device(pci_device_tbl, pci, NULL);
> >
> > /* enable memory mappings */
> > -pci_config_maskw(bdf, PCI_COMMAND, 0,
> > - PCI_COMMAND_IO | PCI_COMMAND_MEMORY |
> > PCI_COMMAND_SERR);
> > +pci_config_maskw_dom(bdf, PCI_COMMAND, 0,
> > + PCI_COMMAND_IO | PCI_COMMAND_MEMORY |
> > PCI_COMMAND_SERR,
> > + pci->domain_nr);
> > /* enable SERR# for forwarding */
> > if (pci->header_type & PCI_HEADER_TYPE_BRIDGE)
> > -pci_config_maskw(bdf, PCI_BRIDGE_CONTROL, 0,
> > - PCI_BRIDGE_CTL_SERR);
> > +pci_config_maskw_dom(bdf, PCI_BRIDGE_CONTROL, 0,
> > + PCI_BRIDGE_CTL_SERR, pci->domain_nr);
> > }
> >
> > -static void pci_bios_init_devices(void)
> > +static void pci_bios_init_devices(int domain_nr)
> > {
> > struct pci_device *pci;
> >
Re: [SeaBIOS] [RFC v2 0/3] Support multiple pci domains in pci_device
Marcel Apfelbaum 于2018年8月25日周六 下午4:07写道： > > Hi, > > On 08/09/2018 08:43 AM, Zihan Yang wrote: > > Currently seabios assumes there is only one pci domain(0), and almost > > everything operates on pci domain 0 by default. This patch aims to add > > multiple pci domain support for pci_device, while reserve the original > > API for compatibility. > > This is a necessary addition to support your QEMU patches, > Please send a link to the QEMU series on your next re-spin. OK, I will attach the link in later QEMU patches. > > The reason to get seabios involved is that the pxb-pcie host bus created > > in QEMU is now in a different PCI domain, and its bus number would start > > from 0 instead of bus_nr. Actually bus_nr should not be used when in > > another non-zero domain. > > That is not necessarily true. As we discussed in QEMU devel > mailing list, it is possible PCI buses of a different domain > to start from a positive bus number. > Both bus_nr and domain_nr support makes sense. OK, I think I will still use bus_nr as the start bus when in separate domain. > > However, QEMU only binds port 0xcf8 and 0xcfc to > > bus pcie.0. To avoid bus confliction, we should use other port pairs for > > busses under new domains. > > I would skip support for IO based configuration and use only MMCONFIG > for extra root buses. > > The question remains: how do we assign MMCONFIG space for > each PCI domain. Indeed, seabios does not have fixed MMCONFIG space(except for q35 host) yet. > Thanks, > Marcel > > > Current issues: > > * when trying to read config space of pcie_pci_bridge, it actually reads > >out the result of mch. I'm working on this weird behavior. > > > > Changelog: > > v2 <- v1: > > - Fix bugs in filtering domains when traversing pci devices > > - Reformat some hardcoded codes, such as probing the pci device in pci_setup > > > > Zihan Yang (3): > >fw/pciinit: Recognize pxb-pcie-dev device > >pci_device: Add pci domain support > >pci: filter undesired domain when traversing pci > > > > src/fw/coreboot.c| 2 +- > > src/fw/csm.c | 2 +- > > src/fw/mptable.c | 1 + > > src/fw/paravirt.c| 3 +- > > src/fw/pciinit.c | 276 > > ++- > > src/hw/ahci.c| 1 + > > src/hw/ata.c | 1 + > > src/hw/esp-scsi.c| 1 + > > src/hw/lsi-scsi.c| 1 + > > src/hw/megasas.c | 1 + > > src/hw/mpt-scsi.c| 1 + > > src/hw/nvme.c| 1 + > > src/hw/pci.c | 69 +++-- > > src/hw/pci.h | 42 +--- > > src/hw/pci_ids.h | 6 +- > > src/hw/pcidevice.c | 11 +- > > src/hw/pcidevice.h | 8 +- > > src/hw/pvscsi.c | 1 + > > src/hw/sdcard.c | 1 + > > src/hw/usb-ehci.c| 1 + > > src/hw/usb-ohci.c| 1 + > > src/hw/usb-uhci.c| 1 + > > src/hw/usb-xhci.c| 1 + > > src/hw/virtio-blk.c | 1 + > > src/hw/virtio-scsi.c | 1 + > > src/optionroms.c | 3 + > > 26 files changed, 268 insertions(+), 170 deletions(-) > > > ___ SeaBIOS mailing list SeaBIOS@seabios.org https://mail.coreboot.org/mailman/listinfo/seabios
