[coreboot-gerrit] Change in coreboot[master]: [WIP] Add Multiple Segment support

[Subrata Banik (Code Review)]

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40722 )

Change subject: [WIP] Add Multiple Segment support
......................................................................

Patch Set 3:
> Patch Set 3:
> > > > > I'm not sure if we have to report these resource
> > > > > again. Maybe we don't need the `extrahostbridge.asl`?
> > > >
> > > > i was referring to Intel RC code for customer where i could find the usage of "extrahostbridge.asl" for additional segment,
> > >
> > > And the authors of that RC code do they have more experience with multiple
> > > PCI segment groups than we have? I don't think anybody gets such things
> > > right on the first try.
> > >
> > > I have looked further into this and disovered the ACPI "Module" device. It
> > > groups multiple devices together and can specify shared resources for them.
> > > Coincidentally, after learning about this, it turned out that the ACPI spec
> > > uses it in the example code for _SEG ;)
> > >
> > > Please have a look at ACPI spec 6.3, `6.5.6.1 Example`. I think this is how
> > > we should do it. Basically:
> > > Device (ND0) {
> > >         Name(_HID, "ACPI0004") /* Module Device */
> > >
> > >         Method(_CRS, ...)
> > >
> > >         Device (PCI0) {
> > >                 ...
> > >         }
> > >
> > >         Device (PCI1) {
> > >                 ...
> > >         }
> > > }
> >
> > Thanks for the point, yes, those references looks good.But i was adhering to Intel RC code which has been tested with this feature
>
> Please elaborate what and how exactly was tested.

TBT devices are functional and any endpoint device sitting is working as expected.
> > https://github.com/otcshare/CCG-TGL-Generic-SiC/tree/master/ClientOneSilicon...
>
> 404
> > In current coreboot pci tree, it appear as below.
> >
> > Scope (_SB) {
>
> ...
> > }
>
> I know, I have reviewed it.
> > Now we can optimize the common resources between PCI0 and PCI1 later and keep side common space for sharing.
>
> It's not an optimization. Pretending that there are two devices
> positively decoding the same resources is simply wrong.

For PCI device access, shoudn't they use same IO and MMIO resources like 0xCF8/0xCFC or MMIO resource via PCIE base address?

why below resource has to be different for 2 different segments ?

//
  // Bus Number Allocation: Bus 0 to 0xFF
  //
  WORDBusNumber(ResourceProducer,MinFixed,MaxFixed,PosDecode,0x00,
    0x0000,0x00FF,0x00,0x0100,,,PB00)

//
  // I/O Region Allocation 0 ( 0x0000 - 0x0CF7 )
  //
  DWordIo(ResourceProducer,MinFixed,MaxFixed,PosDecode,EntireRange,
    0x00,0x0000,0x0CF7,0x00,0x0CF8,,,PI00)

//
  // I/O Region Allocation 1 ( 0x0D00 - 0xFFFF )
  //
  DWordIo(ResourceProducer,MinFixed,MaxFixed,PosDecode,EntireRange,
    0x00,0x0D00,0xFFFF,0x00,0xF300,,,PI01)

//
  // PCI Memory Region ( TOLUD - 0xDFFFFFFF )
  //
  DWordMemory(ResourceProducer,PosDecode,MinFixed,MaxFixed,NonCacheable,
    ReadWrite,0x00,0x00000000,0xDFFFFFFF,0x00,0xE0000000,,,PM01)

//
  // PCI Memory Region ( TOUUD - (TOUUD + ABOVE_4G_MMIO_SIZE) )
  // (This is dummy range for OS compatibility, will patch it in _CRS)
  //
  QWordMemory(ResourceProducer,PosDecode,MinFixed,MaxFixed,NonCacheable,
    ReadWrite,0x00,0x10000,0x1FFFF,0x00,0x10000,,,PM02)

These are generic resources for any PCI segment. Isn't it?
Having those common make sense to avoid duplication else i don't think there might be any reason for not being same.