> -----Original Message-----
> From: Nicolin Chen <[email protected]>
> Sent: 24 November 2025 18:34
> To: Shameer Kolothum <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; Jason Gunthorpe
> <[email protected]>; [email protected]; [email protected]; Nathan
> Chen <[email protected]>; Matt Ochs <[email protected]>;
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> Krishnakant Jaju <[email protected]>
> Subject: Re: [PATCH v6 17/33] hw/arm/smmuv3: Add support for providing a
> direct MSI doorbell GPA
> 
> 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" ?

The  object_property_get_bool(..., &error_abort) is used in
virt-acpi-build.c as well.

Still not sure "NULL" is a good idea or not.

Thanks,
Shameer

Reply via email to