Re: [PATCH v7 3/3] hw/acpi: Add vmclock device

[David Woodhouse]

On Thu, 2025-01-16 at 16:57 +0100, Philippe Mathieu-Daudé wrote:
> On 16/1/25 16:32, David Woodhouse wrote:
> > On Thu, 2025-01-16 at 16:15 +0100, Philippe Mathieu-Daudé wrote:
> > > 
> > > > --- a/hw/acpi/Kconfig
> > > > +++ b/hw/acpi/Kconfig
> > > > @@ -60,6 +60,11 @@ config ACPI_VMGENID
> > > >         default y
> > > >         depends on PC
> > > >     
> > > > +config ACPI_VMCLOCK
> > > > +    bool
> > > > +    default y
> > > > +    depends on PC
> > > 
> > > This doesn't look right (apparently the kernel side also build on ARM).
> > 
> > I don't think it's strictly wrong; there are no circumstances in which
> > PC is set not I386 && ACPI, or vice versa? I was just going from the
> > existing setup for VMGENID, which I think could also theoretically
> > exist on Arm too?
> 
> Unfortunately PC (and MALTA) are bad examples, beeing ones of the
> oldest QEMU machines. Their code is spaghetti.
> 
> ACPI_VMGENID seems over-restricted. IIUC it should be:
> 
>      select ACPI
>      select FW_CFG
> 
> The idea is to keep the smallest dependency, i.e. if someone wants
> to build a binary with only microvm machine and use vmclock in it,
> it shouldn't have to build the PC machines.

I don't think it currently works on microvm, as it has HW reduced ACPI
and the guest doesn't seem to see the SSDT advertising the device.

It looks like this got merged through mst's tree with the original
'depends on PC', which I think isn't strictly wrong even if it isn't
pretty. I'll change that when I get to making vmclock work on microvm
and Arm, if that's OK?

smime.p7s
Description: S/MIME cryptographic signature