Re: [Qemu-devel] [PATCH 2/4] acpi/gpe: expand bits of gpe register

[li guang]

在 2013-05-24五的 10:24 +0900，Isaku Yamahata写道：
> On Wed, May 22, 2013 at 01:37:41PM +0800, li guang wrote:
> > 在 2013-05-22三的 14:28 +0900，Isaku Yamahata写道：
> > > Why?
> > > And it breaks pointer operation like
> > 
> > the fact is I can't guess why gpe->sts is defined uint8_t 
> > but the real hardware is 32-bit width.
> 
> Which section of ACPI spec?

seems ACPI SPEC does not define the length,
but, real hardware does, e.g. ICH9.

> 
> 
> > I expand it to 32 because the future usage for me will 
> > access bit beyond 8.
> > of course, I can keep it, but I don't if I can
> > violate the bit usage indication of this register.
> 
> I don't have strong opinion wheter uint8_t or uint32_t.
> But the current patch is broken as the pointer offset 
> operation needs to be fixed.

Yes, I'll fix later

Thanks!

> 
> > > >          cur = ar->gpe.sts + addr;
> > > 
> > > thanks,
> > > 
> > > On Wed, May 22, 2013 at 11:46:35AM +0800, liguang wrote:
> > > > Signed-off-by: liguang <address@hidden>
> > > > ---
> > > >  hw/acpi/core.c         |    8 ++++----
> > > >  hw/acpi/ich9.c         |    2 +-
> > > >  hw/acpi/piix4.c        |    2 +-
> > > >  include/hw/acpi/acpi.h |    4 ++--
> > > >  4 files changed, 8 insertions(+), 8 deletions(-)
> > > > 
> > > > diff --git a/hw/acpi/core.c b/hw/acpi/core.c
> > > > index 42eeace..38ddeb8 100644
> > > > --- a/hw/acpi/core.c
> > > > +++ b/hw/acpi/core.c
> > > > @@ -575,9 +575,9 @@ void acpi_gpe_reset(ACPIREGS *ar)
> > > >      memset(ar->gpe.en, 0, ar->gpe.len / 2);
> > > >  }
> > > >  
> > > > -static uint8_t *acpi_gpe_ioport_get_ptr(ACPIREGS *ar, uint32_t addr)
> > > > +static uint32_t *acpi_gpe_ioport_get_ptr(ACPIREGS *ar, uint32_t addr)
> > > >  {
> > > > -    uint8_t *cur = NULL;
> > > > +    uint32_t *cur = NULL;
> > > >  
> > > >      if (addr < ar->gpe.len / 2) {
> > > >          cur = ar->gpe.sts + addr;
> > > > @@ -592,7 +592,7 @@ static uint8_t *acpi_gpe_ioport_get_ptr(ACPIREGS 
> > > > *ar, uint32_t addr)
> > > >  
> > > >  void acpi_gpe_ioport_writeb(ACPIREGS *ar, uint32_t addr, uint32_t val)
> > > >  {
> > > > -    uint8_t *cur;
> > > > +    uint32_t *cur;
> > > >  
> > > >      cur = acpi_gpe_ioport_get_ptr(ar, addr);
> > > >      if (addr < ar->gpe.len / 2) {
> > > > @@ -608,7 +608,7 @@ void acpi_gpe_ioport_writeb(ACPIREGS *ar, uint32_t 
> > > > addr, uint32_t val)
> > > >  
> > > >  uint32_t acpi_gpe_ioport_readb(ACPIREGS *ar, uint32_t addr)
> > > >  {
> > > > -    uint8_t *cur;
> > > > +    uint32_t *cur;
> > > >      uint32_t val;
> > > >  
> > > >      cur = acpi_gpe_ioport_get_ptr(ar, addr);
> > > > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> > > > index 4a17f32..582dbec 100644
> > > > --- a/hw/acpi/ich9.c
> > > > +++ b/hw/acpi/ich9.c
> > > > @@ -153,7 +153,7 @@ static int ich9_pm_post_load(void *opaque, int 
> > > > version_id)
> > > >       .info       = &vmstate_info_uint8,                              \
> > > >       .size       = sizeof(uint8_t),                                  \
> > > >       .flags      = VMS_ARRAY | VMS_POINTER,                          \
> > > > -     .offset     = vmstate_offset_pointer(_state, _field, uint8_t),  \
> > > > +     .offset     = vmstate_offset_pointer(_state, _field, uint32_t),  \
> > > >   }
> > > >  
> > > >  const VMStateDescription vmstate_ich9_pm = {
> > > > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> > > > index c4af1cc..3a7b669 100644
> > > > --- a/hw/acpi/piix4.c
> > > > +++ b/hw/acpi/piix4.c
> > > > @@ -205,7 +205,7 @@ static int vmstate_acpi_post_load(void *opaque, int 
> > > > version_id)
> > > >       .info       = &vmstate_info_uint16,                             \
> > > >       .size       = sizeof(uint16_t),                                 \
> > > >       .flags      = VMS_SINGLE | VMS_POINTER,                         \
> > > > -     .offset     = vmstate_offset_pointer(_state, _field, uint8_t),  \
> > > > +     .offset     = vmstate_offset_pointer(_state, _field, uint32_t),  \
> > > >   }
> > > >  
> > > >  static const VMStateDescription vmstate_gpe = {
> > > > diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
> > > > index 635be7b..deca3ce 100644
> > > > --- a/include/hw/acpi/acpi.h
> > > > +++ b/include/hw/acpi/acpi.h
> > > > @@ -112,8 +112,8 @@ struct ACPIPM1CNT {
> > > >  struct ACPIGPE {
> > > >      uint8_t len;
> > > >  
> > > > -    uint8_t *sts;
> > > > -    uint8_t *en;
> > > > +    uint32_t *sts;
> > > > +    uint32_t *en;
> > > >  };
> > > >  
> > > >  struct ACPIREGS {
> > > > -- 
> > > > 1.7.2.5
> > > > 
> > > 
> > 
> > 
>