On Mon, Nov 24, 2025 at 11:01:57AM -0800, Shameer Kolothum wrote:
> > On Mon, Nov 24, 2025 at 12:05:38AM -0800, Shameer Kolothum wrote:
> > > > > > > + if (object_property_find(OBJECT(dev), "accel") &&
> > > > > > > + object_property_get_bool(OBJECT(dev), "accel",
> > &error_abort)) {
> > > > > >
> > > > > > Do we need object_property_find()? A later patch seems to drop it.
> > > > > > Perhaps we shouldn't add it in the first place?
> > > > >
> > > > > We need that at this stage as we haven't added the "accel" property
> > > > > yet
> > > > > and that will cause "make check" tests to fail without that.
> > > > >
> > > > > We remove it once we introduce "accel" property later.
> > > >
> > > > Hmm, I assume object_property_get_bool() would return false when
> > > > "accel" is not available yet? No?
> > >
> > > It is the errp that will be set if there is no "accel" is available that
> > > will fail the
> > > tests. Another way to avoid object_property_find() is to pass NULL for
> > > errp
> > in
> > > object_property_get_bool(), but I think again when we introduce "accel" we
> > have
> > > change that into either errp/error_abort here.
> > >
> > > I am not sure there is any other way to handle this.
> >
> > This path is to set MSI GPA, which could be optional, right? So,
> > we don't really need to exit when accel is not found or accel is
> > set to "n".
> >
> > In that case, could we just use NULL even when "accel" is added?
> > Why do we use error_abort or errp?
>
> I cant recollect why I used error_abort here in the first place. May be
> since we used that for the "primary-bus" above. But as you said,
> we could argue "accel" is optional property. However, if we pass NULL
> and then for some other reason it fails(even if accel=on),
> we will end up ignoring the error completely and silently boot without
> "accel" ?
OK. I see now..
What we want are:
1) When "accel" isn't defined, don't abort it by calling that
object_property_find() to make msi-gpa optional.
2) When "accel" is defined, remove the object_property_find()
because object_property_get_bool() will never abort. And it
will make the msi-gpa optional depending on accel=on/off.
Perhaps we should re-organize the patch sequence, putting the
MSI one later. Checking the "accel" before we introduce it is
a bit odd. But any way, I think we are fine here, if you don't
want to change.
Thanks
Nicolin
