On 2026-09-01 at 17:29 +1000, Danilo Krummrich <[email protected]> wrote... > On Tue Sep 1, 2026 at 7:09 AM CEST, Alistair Popple wrote: > > On 2026-09-01 at 14:50 +1000, Dave Airlie <[email protected]> wrote... > >> > > > >> > > We should make the NovaCoreApi just provide an accessor for &Spec and > >> > > make every > >> > > subsequent method we need public. Otherwise we end up with endless > >> > > forwarding > >> > > methods. We can also add as_raw() methods to the specific types as > >> > > needed. > >> > > >> > Ok. This is where I don't have a good instinct for what we think should > >> > be an > >> > accessor/forwarding method vs. where we should just expose the > >> > underlying data > >> > structure and required methods to API users. > >> > > >> > In the past it seems there's been some resistance to exposing nova-core > >> > or gsp > >> > data structures like this which is why I added the forwarding methods. > >> > In future > >> > we're going to have other data-structures that NovaCoreApi will need to > >> > access > >> > so it would be good to understand what we should do here so we can keep > >> > things > >> > somewhat consistent. > >> > >> We can expose structure defined in nova-core, we cannot expose > >> structures defined in gsp bindings or firmware. > > Yes, those structures are not even exposed to the nova-core layers we are > dealing with here. > > There are the structures abstracting firmware interfaces, but despite exposing > trivial cached values, they should not exposed to nova-drm either. > > Instead nova-drm should call into nova-core and ask it to do things on its > behalf. > > More in general, the same is true for any other device resources. For > instance, > we also should not expose the pci::Device or the pci::Bar to nova-drm, but > again > provide a higher level API. > > > All the fields of self.gpu are structures defined as rust native structures > > in > > nova-core. Their values may be decoded or derived from GSP responses, but by > > design none of the raw structures from gsp bindings live in self.gpu AFAIK. > > We could indeed expose all values from struct Gpu directly and control > everything through visibility, but ... > > >> In theory we can internally between core/drm but I'd really really > >> like to keep that boundary as the limits of GSP for auditability > >> purposes. > > ... exposing struct Gpu and controlling everything through visibility still > means that we could easily miss some device resource being exposed directly to > nova-drm, which also includes GSP abstractions.
The individual fields would still need to be explicitly marked as `pub`, so it's not like just exposing the top-level data structure suddenly exposes all these device resources without them being explicitly marked as accessible. > So, I think we should provide accessors for the major structures that we > intentionally want to expose (so we have an obvious boundary), but not for > every > single fields inside them. Sure, I don't really mind either way. I just figured that's what the point of Rust visibiity was, to provide a simple boiler-plate free way of saying a type/method/field was used externally. But will switch this to using accessors for major data structures instead given that seems preferred. > This is also why I said that we can expose an accessor for struct Spec, which > I > think serves as a good example. Ok, so just to double check are you happy with an accessor to return gsp_static_info as well? Thanks. - Alistair
