Re: [PATCH 02/10] hw/riscv/virt: Add a switch to enable/disable ACPI

[Andrew Jones]

On Tue, Feb 07, 2023 at 11:57:29AM +0800, Bin Meng wrote:
> On Mon, Feb 6, 2023 at 8:36 PM Andrew Jones <ajones@ventanamicro.com> wrote:
> >
> > On Mon, Feb 06, 2023 at 12:18:06PM +0100, Philippe Mathieu-Daudé wrote:
> > > On 6/2/23 11:54, Andrea Bolognani wrote:
> > > > On Thu, Feb 02, 2023 at 10:22:15AM +0530, Sunil V L wrote:
> > > > > +    object_class_property_add(oc, "acpi", "OnOffAuto",
> > > > > +                              virt_get_acpi, virt_set_acpi,
> > > > > +                              NULL, NULL);
> > > > > +    object_class_property_set_description(oc, "acpi",
> > > > > +                                          "Enable ACPI");
> > > >
> > > > The way this works on other architectures (x86_64, aarch64) is that
> > > > you get ACPI by default and can use -no-acpi to disable it if
> > > > desired. Can we have the same on RISC-V, for consistency?
> >
> > Default on, with a user control to turn off, can be done with a boolean.
> > I'm not sure why/if Auto is needed for acpi. Auto is useful when a
> > configuration doesn't support a default setting for a feature. If the
> > user hasn't explicitly requested the feature to be on or off, then the
> > configuration can silently select what works. If, however, the user
> > explicitly chooses what doesn't work, then qemu will fail with an error
> > instead.
> 
> I have a confusion about "OnOffAuto" vs. "bool" type.
> 
> Both "OnOffAuto" vs. "bool" type property can have a default value if
> user does not assign a value to it from command line. The default
> value is:
> 
> - ON_OFF_AUTO_AUTO for "OnOffAuto"
> - false for "bool"
> 
> But the default value can be overridden in the machine's init
> function, like in this patch.
> 
> So I am not really sure how these 2 types of properties are different.
> Why did we introduce a "OnOffAuto" type, and how is that type supposed
> to be used in which scenario?

Auto makes sense for options which have dependencies on other options.
For example, if we have two options, A and B, where A is an Auto and
B is a boolean, then, when A is initialized to AUTO and has a dependency
on B being X, then have the following

        B=X     A=AUTO -> T     (works)
        B=!X    A=AUTO -> F     (works)

(This is the same whether B was set to X by default or explicitly by the
user.)

Now, if the user explicitly sets A, we have

        B=X     A=T             (works)
        B=X     A=F             (works)
        B=!X    A=T             (emit error about B!=X with A=T and fail)
        B=!X    A=F             (works)

We can't have that behavior with A just being a boolean, defaulting to
true, because we don't know if it's true because of the default or
because it was explicitly set

        B=X     A=T             (works, by default or by user)
        B=X     A=F             (works)
        B=!X    A=T             (doesn't work, but if the user didn't
                                 select A=T, then we could have silently
                                 flipped it to F and continued)
        B=!X    A=F             (works)


IOW, Auto just adds one more bit of info, allowing default vs. user
selection to be determined, which can then be used for different
behaviors.

Back to the "acpi" property, I'm not sure what it depends on that
requires it to be an Auto vs. a boolean.

Thanks,
drew