Re: [Qemu-devel] [PATCH v3 03/10] acpi, mem-hotplug: Add acpi_send_gpe_event() to rise sci for memory hotplug.

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Mar 02, 2015 at 10:27:29AM +0100, Igor Mammedov wrote:
> On Sun, 1 Mar 2015 18:29:39 +0100
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Thu, Feb 26, 2015 at 09:16:45AM +0800, Zhu Guihua wrote:
> > > From: Tang Chen <tangchen@cn.fujitsu.com>
> > > 
> > > Add a new API named acpi_send_gpe_event() to send memory hotplug SCI.
> > > Doing this is because this procedure will be used by other functions in the
> > > next coming patches.
> > > 
> > > Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
> > > Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
> > > ---
> > >  hw/acpi/core.c           | 7 +++++++
> > >  hw/acpi/memory_hotplug.c | 6 ++----
> > >  include/hw/acpi/acpi.h   | 3 +++
> > >  3 files changed, 12 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/hw/acpi/core.c b/hw/acpi/core.c
> > > index 51913d6..98ca994 100644
> > > --- a/hw/acpi/core.c
> > > +++ b/hw/acpi/core.c
> > > @@ -666,6 +666,13 @@ uint32_t acpi_gpe_ioport_readb(ACPIREGS *ar, uint32_t addr)
> > >      return val;
> > >  }
> > >  
> > > +void acpi_send_gpe_event(ACPIREGS *ar, qemu_irq irq,
> > > +                         unsigned int hotplug_status)
> > > +{
> > > +    ar->gpe.sts[0] |= hotplug_status;
> > > +    acpi_update_sci(ar, irq);
> > > +}
> > > +
> > >  void acpi_update_sci(ACPIREGS *regs, qemu_irq irq)
> > >  {
> > >      int sci_level, pm1a_sts;
> > > diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> > > index 6d91a0d..5b13baa 100644
> > > --- a/hw/acpi/memory_hotplug.c
> > > +++ b/hw/acpi/memory_hotplug.c
> > > @@ -202,10 +202,8 @@ void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
> > >      mdev->is_enabled = true;
> > >      mdev->is_inserting = true;
> > >  
> > > -    /* do ACPI magic */
> > > -    ar->gpe.sts[0] |= ACPI_MEMORY_HOTPLUG_STATUS;
> > > -    acpi_update_sci(ar, irq);
> > > -    return;
> > > +    /* Do ACPI magic */
> > > +    acpi_send_gpe_event(ar, irq, ACPI_MEMORY_HOTPLUG_STATUS);
> > >  }
> > >  
> > >  static const VMStateDescription vmstate_memhp_sts = {
> > 
> > This is hardly the only place where we change sts[0].
> > 
> > If you are doing this kind of API work, you need
> > to fix it all up.
> I've looked it /i.e. API change/, but it turned out to be rather big
> refactoring hence it's too late for 2.3
> I'll look at it later when 2.4 cycle opens.

I think we don't have a specific schedule for 2.3,
but I don't mind. However, I'd rather either convert everyone, or no
one. Let's not leave codebase in an inconsistent state.
For now open-coding will work fine.


> > 
> > > diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
> > > index 1f678b4..7a0a209 100644
> > > --- a/include/hw/acpi/acpi.h
> > > +++ b/include/hw/acpi/acpi.h
> > > @@ -172,6 +172,9 @@ void acpi_gpe_reset(ACPIREGS *ar);
> > >  void acpi_gpe_ioport_writeb(ACPIREGS *ar, uint32_t addr, uint32_t val);
> > >  uint32_t acpi_gpe_ioport_readb(ACPIREGS *ar, uint32_t addr);
> > >  
> > > +void acpi_send_gpe_event(ACPIREGS *ar, qemu_irq irq,
> > > +                         unsigned int hotplug_status);
> > > +
> > 
> > need to define legal values for hotplug_status.
> > 
> > >  void acpi_update_sci(ACPIREGS *acpi_regs, qemu_irq irq);
> > >  
> > >  /* acpi.c */
> > > -- 
> > > 1.9.3