Re: [SeaBIOS] [PATCH v2 3/3] pci: recognize RH PCI legacy bridge resource reservation capability

[Zihan Yang]

Liu, Jing2  于2018年8月16日周四 上午10:39写道：
> Hi Zihan,
>
> I copy your comments of this patch several days ago here,
> and hope we could continue some discussion in this serial.

Hi Jing,

Sorry for the delay. I tested at the code just now and find that pcie
bridge does not
use a vendor specific capability. I met this warning in another
ongoing patch so I just
added pcie bridge to suppress the warning.

Now that it turns out pcie bridge does not use this vendor specific
capability, I think you
might want to stick with your original patch, and add it in the future
when redhat decides
to add specific capabilities in pcie bridge. I'm not quite an expert
in this field, so unfortunately
I don't have much insights to give, but I'm open for a discussion.
> On 8/16/2018 10:32 AM, Jing Liu wrote:
> [...]
> > 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(1, "PCI: QEMU resource reserve cap vendor ID doesn't match.\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) {
>
> ======= comments by Zihan Yang =========
> I think PCI_DEVICE_ID_REDHAT_PCIE_BRIDGE shoud be added too, in case we put
> a pcie_pci_bridge at the host bus.
> ========================================
> My reply:
> In theory all the pci bridge could add this capability but I'm not sure
> if we really need that? Would like to hear some more suggestion!
>
> Thanks,
> Jing
>
> [...]