Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider working with fwupd? #32

Open
hughsie opened this issue Jan 9, 2023 · 15 comments
Open

Consider working with fwupd? #32

hughsie opened this issue Jan 9, 2023 · 15 comments

Comments

@hughsie
Copy link
Contributor

hughsie commented Jan 9, 2023

Hi Andrew!

Thanks for reaching out on Twitter, this looks exactly like the thing I'm trying to do. I'm really in awe of what you've done in Culvert so far, and I have a feeling I'll have a lot more questions in the future! Some background:

  • fwupd is a system daemon focussed on firmware updates and platform security, and gets metadata and new firmware from the LVFS, another one of the connected projects I maintain. fwupd is installed by default on basically every Linux distro, and also included in ChromeOS -- and the LVFS has supplied over 77 million updates -- so we're doing okay. :)
  • HSI is shorthand for the Host Security ID, which is a set of tests we run on the machine. We're checking for UEFI Secure Boot, the IOMMU being set up correctly, things like various BootGuard straps being configured correctly, and dozens of things more. It's essentially what you've done with Culvert but instead focussing on mainly-consumer Intel/AMD UEFI and some aarch64 bits and getting buy-in from vendors about actually fixing the issues and dragging up the level of security for the platform. It's all documented here: https://fwupd.github.io/libfwupdplugin/hsi.html and it's being expanded all the time with new tests.
  • The fwupd HSI tests are being run at scale with deep integration with the desktop done, Red Hat insights done, and also openSCAP planned as well. We're working with various large, ahem, companies making HSI compliance a bit part of commercial purchasing agreements.
  • A mega vendor want to run fwupd on the BMC itself, initially targetting AST2500 and AST2600 -- and are concerned that OpenBMC/uboot might not doing all it should do. This is why I'm now poking around with the datasheet working out if anything needs to be verified (it does) and if anything exists already (it seems it does!). Several other vendors are using AST2X00 devices and want to verify the BMC security from the host, as the server proprietary BMC stacks are somewhat fragile.

Anyway, this issue is too long already, and is probably actually a discussion -- so if you'd like to move this to email or even a quick zoom call let me know. I'm working for Red Hat in the UK if that helps. Thanks for any ideas or feedback.

@amboar
Copy link
Owner

amboar commented Jan 9, 2023

Hello!

Thanks for the interest. I'll share a bit of background too:

  • I started culvert internally at IBM as a demonstration of the issues outlined in CVE-2019-6260 prior to publication of the CVE, and to push for us (IBM) to mitigate them for Power systems firmware. Its existence lead to @shenki's LCA talk about server security and featured in Eclypsium's write-up of Quanta's BMC firmware woes
  • I've continued to maintain it as it's helped us with BMC and platform bring-up, development and recovery (especially around the OTP strapping in the AST2600), and I expect it will be useful for the same in future generations of BMC SoCs.

I've got some patches in-flight for the AST2600 that might also interest you: master...ast2600 - expect iterations on that series in the days to come :)

Anyway, I'm keen on the idea of integrating culvert's functionality into fwupd. I'll take a look at how fwupd currently implements these tests and have a bit of a think about what the integration path might look like. I'd certainly like your thoughts on that too.

Feel free to email me directly and we can chat that way, and maybe use that to bootstrap a zoom session.

Thanks for getting in touch!

@hughsie
Copy link
Contributor Author

hughsie commented Jan 10, 2023

Hey Andrew,

I was talking to Alex from Eclypsium just a few weeks ago; him and I sync fairly frequently and swap ideas. I'm pretty sure he’s already got a keen eye on fwupd too. :)

Anyway, I'm keen on the idea of integrating culvert's functionality into fwupd

Okay, that would be amazing, but I do see a few potential blockers. Firstly, the licence of fwupd is LGPLv2+, although a few source files from OEMs are dual-licenced, e.g. “LGPLv2+ OR MIT” – I don’t know how you would feel about “LGPLv2+ OR Apache-2.0” if we copy-pasted bits of code from culvert into fwupd. I think the Apache-2.0 and LGPLv2+ licences are broadly the same with a few subtle differences but I’m certainly not an expert in this stuff.

The second is how fwupd needs to run with almost no additional deps – each additional dep means the number of people using the feature goes down two orders of magnitude. So although we can’t depend on things like libfdt -- although we do have a homegrown DeviceTree parser and a FIT parser included in fwupd right now: https://github.com/fwupd/fwupd/blob/main/libfwupdplugin/fu-fdt-firmware.c

The last potential blocker is that we use GObject C in fwupd, rather than “plain” C – so things are objects, signals, properties etc rather that the kernel current style of culvert. e.g. in fwupd parlance, we have a backend that emits device objects with properties, and each test is a HSI attribute with result attributes, rather than the simple (but easy to understand) structure of culvert.

The pragmatic choice might be for fwupd to “steal ideas” from culvert with attribution, and we work together on the high-level tests themselves – maybe even code assuming the license stuff works out of course. I don’t think it makes sense to depend on having a bleeding edge fwupd binary to be able to do low-level AST2x00 bring-up on a new board, for instance -- and so I don't think absorbing culvert is a nice or necessary thing to suggest.

I guess the most common place fwupd runs right now is when it’s running on the host OS, and so we’d need to use LPC & PCIe to talk to the BMC but keep in the back of our minds we’ll need to do the /dev/mem mmap when instead running on the BMC as part of an openBMC build. AFAIK there’s only a rough plan to do that, and it’s not like there’s bazillions of BMCs deployed all running fwupd at the moment. We do have a bazillion instances of fwupd running on servers in the host OS however – which means I should probably focus on that use case first.

I’d also like to know of the best way to play with this stuff; I have an expensive Lenovo server here with ASPEED Pilot 4 BMC, but I’m guessing those are not going to be supported in the same way. I’ve also got a evb-ast2500 on my desk which we used for the fwupd-on-BMC demo – which works with the /dev/mem bridge, but not the others for obvious reasons. e.g. could I expense something inexpensive like https://www.ebay.co.uk/itm/325476323623 and use that for testing fwupd?

I guess one thing we could do right now is actually flesh out the culvert probe stuff to include a ton more data, perhaps even creating HSI attributes or a JSON export for each of them. For instance, creating files like https://github.com/fwupd/fwupd/blob/main/docs/hsi-tests.d/org.fwupd.hsi.Mei.ManufacturingMode.json that get built into the docs like https://fwupd.github.io/libfwupdplugin/hsi.html#me-not-in-manufacturing-mode make this stuff into an official standard we can point vendors at – but you’re obviously the expert here.

I guess one first action for me would be to move the fdt-parsing code out of the existing vbe plugin and move it lower in the stack so that different plugins can match the compatible strings and create the different device instance types in different places. Regardless of the BMC work that probably makes sense, so I'll give that a go this week.

@amboar
Copy link
Owner

amboar commented Jan 10, 2023

Quite a bit to work through there :)

I took a look at the fwupd codebase after you opened the issue and I tend to agree the way forward is one of two extremes:

  1. Your "steal ideas" approach - re-implement the functionality in fwupd and lean on the abstractions I've put together in culvert as a guide
  2. Ship culvert as a binary and fork/exec it, and e.g. parsing results from stdout

I'm not sure how 2 plays into your "each additional dep means the number of people using the feature goes down two orders of magnitude" issue - arguably it would be a dependency, but not in a library sense. I'm not sure if the distinction matters. Shipping culvert as a separate binary avoids us having to actually resolve the licensing question, and we could add meson options to trim functionality down to meet the requirements of fwupd. Adding a HSI reporting mode to culvert would be pretty neat, though from what I've seen the HSI JSON documents you have are more about enumerating and describing the attributes rather than measuring? Do you have a schema for measurement results? Regardless, I think we could cook up some descriptions based on the text in CVE-2019-6260.

1 is obviously more work, but gives a much tighter integration into fwupd and restricts the scope of behaviour to at most fill fwupd's requirements. Was flashing a new BMC image ever a goal (asking due to some of the discussion on Mastodon)? There's obviously tension there between measuring the bridge state to assess security posture and requiring the bridges be available for flashing purposes, bypassing any authentication/authorisation on the BMC's part.

As for playing with stuff, I think the other thread on Mastodon is going to be the most help there. Aspeed are also quite responsive on the OpenBMC Discord and might be able to give you some pointers (maybe it's possible to rig up an EVB in a useful configuration - I don't have one at hand).

Anyway, I'm happy to help out either way where I can!

@hughsie
Copy link
Contributor Author

hughsie commented Jan 11, 2023

Quite a bit to work through there :)

Ya, sorry! I'm passionate about this firmware stuff, I could talk for hours about this stuff :)

Shipping culvert as a separate binary

I'm less keen on this; various vendors have tried to convince us to exec random binaries for enumeration, firmware writing and security validation, and I think in all honesty this would be horrific from a "keeping it all working in CI" point of view, and greatly complicates things like project maintenance, device lifecycle, caching and resource locking. It also makes progress reporting almost impossible and also selinux policy much harder to satisfy. I think it would be a bad precedent and would open the floodgates tbh.

Was flashing a new BMC image ever a goal

Nobody has asked for it, but if we support 69 different (literally) ways to flash hardware, one more is no biggie.

the HSI JSON documents you have are more about enumerating and describing the attributes rather than measuring

We do both, and even try to fix the problems if that's possible. Obviously not in this case, but it's at least an option. e.g.

Screenshot from 2023-01-11 15-35-48

Screenshot from 2023-01-11 15-36-22

More interesting for stuff like Red Hat Insights is the JSON output, which culvert could certainly copy, e.g.

Screenshot from 2023-01-11 15-37-27

Anyway, I'm happy to help out either way where I can!

I'd appreciate your edits and comments on https://docs.google.com/document/d/1Ib_1hwxCnfe-H0d7jeBOi38NYyTHIzEi66OcU61xLY4/edit?usp=sharing -- I think it makes a lot of sense to agree on the failures and the severities. Maybe some of those don't make sense as HSI attrs, ideas welcome.

@amboar
Copy link
Owner

amboar commented Jan 11, 2023

I'm less keen on this; various vendors have tried to convince us to exec random binaries for enumeration, firmware writing and security validation, and I think in all honesty this would be horrific from a "keeping it all working in CI" point of view, and greatly complicates things like project maintenance, device lifecycle, caching and resource locking. It also makes progress reporting almost impossible and also selinux policy much harder to satisfy. I think it would be a bad precedent and would open the floodgates tbh.

Yeah, many good concerns there. Let's concentrate on getting the bits we need implemented directly in fwupd.

More interesting for stuff like Red Hat Insights is the JSON output, which culvert could certainly copy, e.g.

Right, that's the output that I was asking about. From the brief look I took at the JSON documents in fwupd.git it wasn't obvious that e.g. HsiResult was specified.

I'd appreciate your edits and comments on https://docs.google.com/document/d/1Ib_1hwxCnfe-H0d7jeBOi38NYyTHIzEi66OcU61xLY4/edit?usp=sharing -- I think it makes a lot of sense to agree on the failures and the severities. Maybe some of those don't make sense as HSI attrs, ideas welcome.

Yep, let me have a read through and provide some feedback.

@amboar
Copy link
Owner

amboar commented Jan 12, 2023

Okay, I've left a bunch of comments on the google doc. Let me know what you think.

@hughsie
Copy link
Contributor Author

hughsie commented Jan 12, 2023

Yeah, many good concerns there. Let's concentrate on getting the bits we need implemented directly in fwupd.

I think a good starting point would be to add the checks for the devmem bridge, e.g. Read DWORD HICRB (0x100h) and check that bit 6 is set -- I've added these sections to the doc -- any help there most welcome. If I understand the culvert code correctly the devmem bridge isn't doing any of the report checks either -- i.e. we need the functionality there too.

@hughsie
Copy link
Contributor Author

hughsie commented Jan 12, 2023

Yeah, many good concerns there. Let's concentrate on getting the bits we need implemented directly in fwupd.

I think a good starting point would be to add the checks for the devmem bridge, e.g. Read DWORD HICRB (0x100h) and check that bit 6 is set -- I've added these sections to the doc -- any help there most welcome. If I understand the culvert code correctly the devmem bridge isn't doing any of the report checks either -- i.e. we need the functionality there too.

I'm somewhat sceptical that server OSs are going to let us use all the bridges -- i.e. with kernel lockdown enabled the iopl() and outb_p() code is going to fail. I don't know if remapping the MMIO BARs is going to work, but I guess it's more likely than the SIO method. I also think matching the AST_PCI_VID&AST_PCI_DID_VGA PCI device is going to be a lot safer than just doing SIO writes on unknown hardware -- I don't know if 0xA5A5 is a common or unique unlock code. Wisdom is certainly appreciated.

@amboar
Copy link
Owner

amboar commented Jan 13, 2023

I think a good starting point would be to add the checks for the devmem bridge, e.g. Read DWORD HICRB (0x100h) and check that bit 6 is set -- I've added these sections to the doc -- any help there most welcome.

Right, let me fill those out. It made me think about whether there's a better way to describe all the bits so that you could dump them out as reference. That would make your job here a bit easier. However, I think it would require a bit of rearchitecting as I've encoded this info in the code rather than defining it statically.

If I understand the culvert code correctly the devmem bridge isn't doing any of the report checks either -- i.e. we need the functionality there too.

Can you clarify what you mean here? Is it that you'd like to see a report of whether the /dev/mem interface is available in e.g. the output of probe? There are limitations there...

I'm somewhat sceptical that server OSs are going to let us use all the bridges -- i.e. with kernel lockdown enabled the iopl() and outb_p() code is going to fail. I don't know if remapping the MMIO BARs is going to work, but I guess it's more likely than the SIO method. I also think matching the AST_PCI_VID&AST_PCI_DID_VGA PCI device is going to be a lot safer than just doing SIO writes on unknown hardware -- I don't know if 0xA5A5 is a common or unique unlock code. Wisdom is certainly appreciated.

Yeah, all valid concerns. The latter concern is the reason for the warning in caps at the top of the README :)

Getting back to iopl() and outb_p(), you'll be pleased to know that the SoCs implement a PCI-to-LPC bridge :) However, whether it's enabled is under control of the BMC, so it may not be enabled in all circumstances and is documented to be disabled by default. But if it is enabled, we can use the PCIe BAR mapping to drive LPC device accesses rather than inb/outb.

Stepping back for a moment, I guess we need to be explicit about what we're trying to establish. Are we trying to establish whether the BMC is vulnerable with no assumptions about the access method on the host side (e.g. bare-metal hosting or host kernel compromise are in-scope), or are we trying to establish vulnerability to the host userspace (assuming root)? I presume the former. If the host kernel is locking down userspace access then I can't see how we can escape having help from the kernel to establish the presence of the vulnerabilities.

@amboar
Copy link
Owner

amboar commented Jan 13, 2023

I've added a table describing all the bits in each SoC's physical address space for org.fwupd.hsi.Aspeed.iLPC2AHB.ReadWrite. Let me know what you think. If it's helpful we can discuss how we go about adding the table for the remaining sections.

@hughsie
Copy link
Contributor Author

hughsie commented Jan 13, 2023

Is it that you'd like to see a report of whether the /dev/mem interface is available

Nah, I mean if you do culvert probe when running on the SoC device it should show the user what the problems are.

we can use the PCIe BAR mapping to drive LPC device accesses

Can you remap the BAR when the kernel is locked down?

Are we trying to establish whether the BMC is vulnerable

For the first cut, I want to run fwupd on the BMC as part of OpenBMC and then output a report on what problems there are. I think working on the PCI-to-LPC bit so we can scan from the host OS is probably phase 2.

I've added a table describing all the bits in each SoC's physical address space

I think that's a really good start, more certainly welcome! I've got some test code that should be ready for some rough review next week -- i.e. mmap /dev/mem, read 0x1e6e2070 and look for the bit etc. i.e. a really simple first cut.

@amboar
Copy link
Owner

amboar commented Jan 13, 2023

Nah, I mean if you do culvert probe when running on the SoC device it should show the user what the problems are.

This should work. Is it not working for you? I just checked on an AST2600 and it provides this output:

root@ast2600:~# ./culvert -v probe                                                                                                                                                                                                                                                                                                                     
[*] Found 5 registered bridge drivers
[*] Trying bridge driver Debug UART
[*] Unrecognised argument list for debug interface (0)
[*] Trying bridge driver devmem
[*] Probing devmem
[*] Probing for SoC revision registers
[*] Found revision 0x5030303
[*] Trying bridge driver iLPC2AHB
[*] Failed to initialise iLPC bridge: -95
[*] Trying bridge driver LPC2AHB
[*] Failed to initialise L2A bridge: -95
[*] Trying bridge driver P2A
[*] Failed to initialise P2A bridge: -2
[*] Probing for SoC revision registers
[*] Found revision 0x5030303
[*] Selected devicetree for SoC 'aspeed,ast2600'
[*] Found 14 registered drivers
[*] Bound trace driver to /ahb/bus-controller@1e600000
[*] Bound sdmc driver to /ahb/apb/memory-controller@1e6e0000
[*] Bound strap driver to /ahb/apb/syscon@1e6e2000/strapping
[*] Bound sioctl driver to /ahb/apb/syscon@1e6e2000/superio
[*] Bound bridge-controller driver to /ahb/apb/syscon@1e6e2000/bridge-controller
[*] Bound pciectl driver to /ahb/apb/syscon@1e6e2000/pcie-bridge-controller
[*] Bound otp driver to /ahb/apb/secure-boot-controller@1e6f2000
[*] Bound debugctl driver to /ahb/apb/uart@1e783000
[*] Bound debugctl driver to /ahb/apb/uart@1e784000
[*] Bound vuart driver to /ahb/apb/serial@1e787000
[*] Bound vuart driver to /ahb/apb/serial@1e788000
[*] Bound ilpcctl driver to /ahb/apb/lpc@1e789000/bridge-controller
[*] Initialised strap driver
[*] Initialised sioctl driver
[*] Initialised bridge-controller driver
[*] iLPC bridge gate ID: 5
[*] Initialised ilpcctl driver
[*] Initialised ilpcctl AHB bridge controller
[*] Initialised debugctl driver
[*] Initialised debugctl AHB bridge controller
[*] Initialised debugctl driver
[*] Initialised debugctl AHB bridge controller
[*] fdt: Searching devicetree for type 'memory'
[*] Initialised sdmc driver
[*] Initialised pciectl driver
[*] Initialised pciectl AHB bridge controller
xdma:   Restricted
        BMC: Enabled
        XDMA on BMC: Enabled
        VGA: Disabled
        XDMA is constrained: Yes
p2a:    Disabled
debug:  Permissive
        Debug UART port: UART1
debug:  Permissive
        Debug UART port: UART5
ilpc:   Disabled
[*] Unbound instance of driver ilpcctl
[*] Unbound instance of driver vuart
[*] Unbound instance of driver vuart
[*] Unbound instance of driver debugctl
[*] Unbound instance of driver debugctl
[*] Unbound instance of driver otp
[*] Unbound instance of driver pciectl
[*] Unbound instance of driver bridge-controller
[*] Unbound instance of driver sioctl
[*] Unbound instance of driver strap
[*] Unbound instance of driver sdmc
[*] Unbound instance of driver trace

Can you remap the BAR when the kernel is locked down?

I haven't poked much at lockdown tbh so I went looking. It seems you can't map the BAR into userspace when lockdown is sufficiently enforced:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/pci-sysfs.c?h=v6.1#n1028

For the first cut, I want to run fwupd on the BMC as part of OpenBMC and then output a report on what problems there are. I think working on the PCI-to-LPC bit so we can scan from the host OS is probably phase 2.

Sounds good.

I think that's a really good start, more certainly welcome! I've got some test code that should be ready for some rough review next week -- i.e. mmap /dev/mem, read 0x1e6e2070 and look for the bit etc. i.e. a really simple first cut.

Okay. Given you listed BMC and host access separate I wasn't quite sure whether that table was exactly what you were after. Since it's at least useful, we can work to add that table for the remaining HSIs.

As a point of reference, anything that calls soc_bridge_controller_register() is a driver that controls the state of a bridge. The bits we need to touch are (in)directly referenced from the enforce() callback implementation. This might end up calling into other areas of the codebase. From there it's a bit of a maze of mapping the compatibles back to the devicetrees to derive the absolute register addresses. The list isn't terribly long (though eventually needs to account for the eSPI and LPC2AHB bridges):

$ git grep soc_bridge_controller_register
src/soc.c:soc_bridge_controller_register(struct soc *ctx, struct bridgectl *bridge, const struct bridgectl_ops *ops)
src/soc.h:int soc_bridge_controller_register(struct soc *soc, struct bridgectl *bridge,
src/soc/debugctl.c:    if ((rc = soc_bridge_controller_register(soc, debugctl_as_bridgectl(ctx), ops)) < 0) {
src/soc/ilpcctl.c:    if ((rc = soc_bridge_controller_register(soc, ilpcctl_as_bridgectl(ctx), ops)) < 0) {
src/soc/pciectl.c:    if ((rc = soc_bridge_controller_register(soc, p2actl_as_bridgectl(ctx), &p2actl_ops)) < 0) {
src/soc/pciectl.c:    if ((rc = soc_bridge_controller_register(soc, xdmactl_as_bridgectl(ctx), &xdmactl_ops)) < 0) {

So if you want to get a head-start you can dig in that way, otherwise I'll plug away at adding the tables when I have a moment.

@hughsie
Copy link
Contributor Author

hughsie commented Jan 14, 2023

[*] Initialised pciectl AHB bridge controller

Aha, I didn't know that would work when local. Isn't devmem going to work in all cases, and possibly be a more reliable way to retrieve the information? Or do you think we should always be using pciectl in fwupd when running on the BMC rather than devmem?

@amboar
Copy link
Owner

amboar commented Jan 15, 2023

Ah, I think we're running up against two things here:

  1. Naming things is hard
  2. My absolute lack of any design or usage documentation

Let me try to fix 2 properly, however as a brief explanation the codebase is split into approximately three parts:

a. The stuff under src/bridge
b. The stuff under src/devicetree
c. The stuff under src/soc
d. Everything else (logging, host bits that are dependencies of the stuff under src/bridge, etc)

Considering a. above, the source under src/bridge contains all the host-side drivers for the bridges ("host" is used a bit loosely here, as it also contains the /dev/mem driver for use on the BMC - I consider the machine on which culvert is running to be called the "host" rather than the processor that's attached to the BMC):

$ git ls-files src/bridge
src/bridge/debug.c
src/bridge/debug.h
src/bridge/devmem.c
src/bridge/devmem.h
src/bridge/ilpc.c
src/bridge/ilpc.h
src/bridge/l2a.c
src/bridge/l2a.h
src/bridge/meson.build
src/bridge/p2a.c
src/bridge/p2a.h

Host bridge drivers are registered in the code via the REGISTER_BRIDGE_DRIVER() macro. Their role on initialisation is to produce an AHB object (struct ahb) on which we can perform {read,write}{,l}() operations. This gives us an abstraction that we exploit through the rest of the codebase, where we don't care which bridge we're using, only that we can perform reads and writes on the AHB.

Reads and writes on the AHB are handy, however, what's even more handy is pairing this capability with the knowledge of how the AHB address space is laid out. The layout of each SoC's AHB is described by the devicetrees in src/devicetree. A constructed SoC object (struct soc) emerges from the soc_from_rev(..., rev_probe(ahb)) sequence in soc_probe() once we have our struct ahb and combines the appropriate devicetree with the struct ahb instance. From there we can start binding drivers to devices in the AHB address space and doing interesting things.

The drivers for the devices in the SoC's address space live in src/soc and are registered with the REGISTER_SOC_DRIVER() macro. They are all implemented in terms of struct soc, which gives them an abstract mechanism to drive reads and writes on the AHB (via struct soc's struct ahb instance). Returning to the bridges for a moment, it's the case that the SoC exposes control bits for them on the APB address space (mapped into the AHB). Given that a lot of the interest here is about inspecting and controlling the state of these bridges, there are drivers that do just that in src/soc:

$ git grep soc_bridge_controller_register
src/soc.c:soc_bridge_controller_register(struct soc *ctx, struct bridgectl *bridge, const struct bridgectl_ops *ops)
src/soc.h:int soc_bridge_controller_register(struct soc *soc, struct bridgectl *bridge,
src/soc/debugctl.c:    if ((rc = soc_bridge_controller_register(soc, debugctl_as_bridgectl(ctx), ops)) < 0) {
src/soc/ilpcctl.c:    if ((rc = soc_bridge_controller_register(soc, ilpcctl_as_bridgectl(ctx), ops)) < 0) {
src/soc/pciectl.c:    if ((rc = soc_bridge_controller_register(soc, p2actl_as_bridgectl(ctx), &p2actl_ops)) < 0) {
src/soc/pciectl.c:    if ((rc = soc_bridge_controller_register(soc, xdmactl_as_bridgectl(ctx), &xdmactl_ops)) < 0) {

Enter the naming problem :) I needed some way to differentiate between e.g. ilpc as a host bridge driver and ilpc as a SoC bridge driver. I probably could have chosen a better scheme, but in terms of the symbols in the code the SoC-side bridge drivers are suffixed with ctl.

Rounding it out, soc_bridge_controller_register() gathers all instances of SoC-side bridge drivers into a list for us to work with later on in the execution of culvert.

With this structure it's the case that however we drive accesses on the BMC AHB we can easily inspect the configuration of all the bridges, and hence culvert probe works over /dev/mem, the P2A, the iLPC bridge, and the debug UART equally.

Finally, returning to your comment:

[*] Initialised pciectl AHB bridge controller

Aha, I didn't know that would work when local. Isn't devmem going to work in all cases, and possibly be a more reliable way to retrieve the information? Or do you think we should always be using pciectl in fwupd when running on the BMC rather than devmem?

From the above the output [*] Initialised pciectl AHB bridge controller comes from the driver for the SoC-side PCIe device controller (src/soc/pciectl.c), i.e. the driver that encodes which bits to inspect to determine how the various PCIe devices and bridges are configured (VGA and BMC devices, P2A and XDMA bridges) rather than the host-side P2A bridge driver (src/bridge/p2a.c). Earlier on in the output we can see the host-side P2A bridge driver (src/bridge/p2a.c) fail to initialise:

...
[*] Trying bridge driver P2A
[*] Failed to initialise P2A bridge: -2
...

Does that help?

@amboar
Copy link
Owner

amboar commented Jan 16, 2023

Okay, I've added the rest of the tables and covered the interesting configuration bits for all of the AST2400, AST2500 and AST2600 in the shared google doc. That should be a decent start.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants