On 2026-09-02 at 15:21 +1000, Alistair Popple <[email protected]> wrote... > 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.
That said with the changes required for Invariant Gpu and being able to project Covariant fields out I can see why accessors for top-level data structures might be useful, particularly if some of them (eg. Spec) are Covariant. > > 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? Still interested in your thoughts here though, just to avoid churn because I'll probably pick the wrong answer :-) - Alistair > Thanks. > > - Alistair
