* [Qemu-devel] [PATCH] fix acpi regression @ 2011-04-12 5:13 Wen Congyang 2011-04-12 8:48 ` Isaku Yamahata 2011-04-12 9:27 ` [Qemu-devel] [PATCH V2] " Wen Congyang 0 siblings, 2 replies; 8+ messages in thread From: Wen Congyang @ 2011-04-12 5:13 UTC (permalink / raw) To: Isaku Yamahata, Aurelien Jarno, qemu-devel, blauwirbel This bug is introduced by commit 23910d3f. Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> --- hw/acpi.c | 10 +++------- 1 files changed, 3 insertions(+), 7 deletions(-) diff --git a/hw/acpi.c b/hw/acpi.c index e372474..790bd3b 100644 --- a/hw/acpi.c +++ b/hw/acpi.c @@ -355,7 +355,7 @@ static uint8_t *acpi_gpe_ioport_get_ptr(ACPIGPE *gpe, uint32_t addr) if (addr < gpe->len / 2) { cur = gpe->sts + addr; } else if (addr < gpe->len) { - cur = gpe->en + addr; + cur = gpe->en + addr - gpe->len / 2; } else { abort(); } @@ -369,12 +369,8 @@ void acpi_gpe_ioport_writeb(ACPIGPE *gpe, uint32_t addr, uint32_t val) addr -= gpe->blk; cur = acpi_gpe_ioport_get_ptr(gpe, addr); - if (addr < gpe->len / 2) { - /* GPE_STS */ - *cur = (*cur) & ~val; - } else if (addr < gpe->len) { - /* GPE_EN */ - *cur = val; + if (cur != NULL) { + *cur = val & 0xff; } else { abort(); } -- 1.7.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] fix acpi regression 2011-04-12 5:13 [Qemu-devel] [PATCH] fix acpi regression Wen Congyang @ 2011-04-12 8:48 ` Isaku Yamahata 2011-04-12 9:08 ` Wen Congyang 2011-04-12 9:27 ` [Qemu-devel] [PATCH V2] " Wen Congyang 1 sibling, 1 reply; 8+ messages in thread From: Isaku Yamahata @ 2011-04-12 8:48 UTC (permalink / raw) To: Wen Congyang; +Cc: blauwirbel, qemu-devel, Aurelien Jarno On Tue, Apr 12, 2011 at 01:13:15PM +0800, Wen Congyang wrote: > This bug is introduced by commit 23910d3f. Oh, Thanks. Good catch. I agree with the first hunk. But what is the second hunk for? The GPE STS register is W1C. (NULL check and 0xff masking is okay. it's a bit redundant, though) >From ACPI4 spec 4.7.4.1.1. can only be cleared by software writing a "1" to its respective bit position. thanks, > > Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> > > --- > hw/acpi.c | 10 +++------- > 1 files changed, 3 insertions(+), 7 deletions(-) > > diff --git a/hw/acpi.c b/hw/acpi.c > index e372474..790bd3b 100644 > --- a/hw/acpi.c > +++ b/hw/acpi.c > @@ -355,7 +355,7 @@ static uint8_t *acpi_gpe_ioport_get_ptr(ACPIGPE *gpe, uint32_t addr) > if (addr < gpe->len / 2) { > cur = gpe->sts + addr; > } else if (addr < gpe->len) { > - cur = gpe->en + addr; > + cur = gpe->en + addr - gpe->len / 2; > } else { > abort(); > } > @@ -369,12 +369,8 @@ void acpi_gpe_ioport_writeb(ACPIGPE *gpe, uint32_t addr, uint32_t val) > > addr -= gpe->blk; > cur = acpi_gpe_ioport_get_ptr(gpe, addr); > - if (addr < gpe->len / 2) { > - /* GPE_STS */ > - *cur = (*cur) & ~val; > - } else if (addr < gpe->len) { > - /* GPE_EN */ > - *cur = val; > + if (cur != NULL) { > + *cur = val & 0xff; > } else { > abort(); > } > -- > 1.7.1 > -- yamahata ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] fix acpi regression 2011-04-12 8:48 ` Isaku Yamahata @ 2011-04-12 9:08 ` Wen Congyang 2011-04-12 9:20 ` Isaku Yamahata 0 siblings, 1 reply; 8+ messages in thread From: Wen Congyang @ 2011-04-12 9:08 UTC (permalink / raw) To: Isaku Yamahata; +Cc: blauwirbel, qemu-devel, Aurelien Jarno At 04/12/2011 04:48 PM, Isaku Yamahata Write: > On Tue, Apr 12, 2011 at 01:13:15PM +0800, Wen Congyang wrote: >> This bug is introduced by commit 23910d3f. > > Oh, Thanks. Good catch. I agree with the first hunk. > But what is the second hunk for? The GPE STS register is W1C. > (NULL check and 0xff masking is okay. it's a bit redundant, though) I found this bug when I hotplug a nic. I do not know about ACPI. The second hunk is the same before commit 23910d3f. I will read ACPI spec to confirm it and update this patch. Thanks > >>From ACPI4 spec 4.7.4.1.1. > can only be cleared by software writing a "1" to its respective bit position. > > thanks, > >> >> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> >> >> --- >> hw/acpi.c | 10 +++------- >> 1 files changed, 3 insertions(+), 7 deletions(-) >> >> diff --git a/hw/acpi.c b/hw/acpi.c >> index e372474..790bd3b 100644 >> --- a/hw/acpi.c >> +++ b/hw/acpi.c >> @@ -355,7 +355,7 @@ static uint8_t *acpi_gpe_ioport_get_ptr(ACPIGPE *gpe, uint32_t addr) >> if (addr < gpe->len / 2) { >> cur = gpe->sts + addr; >> } else if (addr < gpe->len) { >> - cur = gpe->en + addr; >> + cur = gpe->en + addr - gpe->len / 2; >> } else { >> abort(); >> } >> @@ -369,12 +369,8 @@ void acpi_gpe_ioport_writeb(ACPIGPE *gpe, uint32_t addr, uint32_t val) >> >> addr -= gpe->blk; >> cur = acpi_gpe_ioport_get_ptr(gpe, addr); >> - if (addr < gpe->len / 2) { >> - /* GPE_STS */ >> - *cur = (*cur) & ~val; >> - } else if (addr < gpe->len) { >> - /* GPE_EN */ >> - *cur = val; >> + if (cur != NULL) { >> + *cur = val & 0xff; >> } else { >> abort(); >> } >> -- >> 1.7.1 >> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] fix acpi regression 2011-04-12 9:08 ` Wen Congyang @ 2011-04-12 9:20 ` Isaku Yamahata 2011-04-12 9:26 ` Wen Congyang 0 siblings, 1 reply; 8+ messages in thread From: Isaku Yamahata @ 2011-04-12 9:20 UTC (permalink / raw) To: Wen Congyang; +Cc: blauwirbel, qemu-devel, Aurelien Jarno On Tue, Apr 12, 2011 at 05:08:18PM +0800, Wen Congyang wrote: > At 04/12/2011 04:48 PM, Isaku Yamahata Write: > > On Tue, Apr 12, 2011 at 01:13:15PM +0800, Wen Congyang wrote: > >> This bug is introduced by commit 23910d3f. > > > > Oh, Thanks. Good catch. I agree with the first hunk. > > But what is the second hunk for? The GPE STS register is W1C. > > (NULL check and 0xff masking is okay. it's a bit redundant, though) > > I found this bug when I hotplug a nic. > I do not know about ACPI. > The second hunk is the same before commit 23910d3f. > I will read ACPI spec to confirm it and update this patch. You mean gpe_writeb(), gpe_write_val(), gpe_reset_val()? gpe_reset_val() does W1C. The GPE0_LEN of piix4 is 4 byte length and the first 2 registers are sts register. From the old code, static void gpe_reset_val(uint16_t *cur, int addr, uint32_t val) { uint16_t x1, x0 = val & 0xff; int shift = (addr & 1) ? 8 : 0; x1 = (*cur >> shift) & 0xff; x1 = x1 & ~x0; <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< W1C *cur = (*cur & (0xff << (8 - shift))) | (x1 << shift); } static void gpe_writeb(void *opaque, uint32_t addr, uint32_t val) { PIIX4PMState *s = opaque; struct gpe_regs *g = &s->gpe; switch (addr) { case GPE_BASE: case GPE_BASE + 1: gpe_reset_val(&g->sts, addr, val); break; thanks, > > Thanks > > > > >>From ACPI4 spec 4.7.4.1.1. > > can only be cleared by software writing a "1" to its respective bit position. > > > > thanks, > > > >> > >> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> > >> > >> --- > >> hw/acpi.c | 10 +++------- > >> 1 files changed, 3 insertions(+), 7 deletions(-) > >> > >> diff --git a/hw/acpi.c b/hw/acpi.c > >> index e372474..790bd3b 100644 > >> --- a/hw/acpi.c > >> +++ b/hw/acpi.c > >> @@ -355,7 +355,7 @@ static uint8_t *acpi_gpe_ioport_get_ptr(ACPIGPE *gpe, uint32_t addr) > >> if (addr < gpe->len / 2) { > >> cur = gpe->sts + addr; > >> } else if (addr < gpe->len) { > >> - cur = gpe->en + addr; > >> + cur = gpe->en + addr - gpe->len / 2; > >> } else { > >> abort(); > >> } > >> @@ -369,12 +369,8 @@ void acpi_gpe_ioport_writeb(ACPIGPE *gpe, uint32_t addr, uint32_t val) > >> > >> addr -= gpe->blk; > >> cur = acpi_gpe_ioport_get_ptr(gpe, addr); > >> - if (addr < gpe->len / 2) { > >> - /* GPE_STS */ > >> - *cur = (*cur) & ~val; > >> - } else if (addr < gpe->len) { > >> - /* GPE_EN */ > >> - *cur = val; > >> + if (cur != NULL) { > >> + *cur = val & 0xff; > >> } else { > >> abort(); > >> } > >> -- > >> 1.7.1 > >> > > -- yamahata ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] fix acpi regression 2011-04-12 9:20 ` Isaku Yamahata @ 2011-04-12 9:26 ` Wen Congyang 0 siblings, 0 replies; 8+ messages in thread From: Wen Congyang @ 2011-04-12 9:26 UTC (permalink / raw) To: Isaku Yamahata; +Cc: blauwirbel, qemu-devel, Aurelien Jarno At 04/12/2011 05:20 PM, Isaku Yamahata Write: > On Tue, Apr 12, 2011 at 05:08:18PM +0800, Wen Congyang wrote: >> At 04/12/2011 04:48 PM, Isaku Yamahata Write: >>> On Tue, Apr 12, 2011 at 01:13:15PM +0800, Wen Congyang wrote: >>>> This bug is introduced by commit 23910d3f. >>> >>> Oh, Thanks. Good catch. I agree with the first hunk. >>> But what is the second hunk for? The GPE STS register is W1C. >>> (NULL check and 0xff masking is okay. it's a bit redundant, though) >> >> I found this bug when I hotplug a nic. >> I do not know about ACPI. >> The second hunk is the same before commit 23910d3f. >> I will read ACPI spec to confirm it and update this patch. > > You mean gpe_writeb(), gpe_write_val(), gpe_reset_val()? > gpe_reset_val() does W1C. > The GPE0_LEN of piix4 is 4 byte length and the first 2 registers are > sts register. From the old code, > > static void gpe_reset_val(uint16_t *cur, int addr, uint32_t val) > { > uint16_t x1, x0 = val & 0xff; > int shift = (addr & 1) ? 8 : 0; > > x1 = (*cur >> shift) & 0xff; > > x1 = x1 & ~x0; <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< W1C > > *cur = (*cur & (0xff << (8 - shift))) | (x1 << shift); > } > > > static void gpe_writeb(void *opaque, uint32_t addr, uint32_t val) > { > PIIX4PMState *s = opaque; > struct gpe_regs *g = &s->gpe; > > switch (addr) { > case GPE_BASE: > case GPE_BASE + 1: > gpe_reset_val(&g->sts, addr, val); > break; Ah, you are right. Thanks. > > thanks, > >> >> Thanks >> >>> >>> >From ACPI4 spec 4.7.4.1.1. >>> can only be cleared by software writing a "1" to its respective bit position. >>> >>> thanks, >>> >>>> >>>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> >>>> >>>> --- >>>> hw/acpi.c | 10 +++------- >>>> 1 files changed, 3 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/hw/acpi.c b/hw/acpi.c >>>> index e372474..790bd3b 100644 >>>> --- a/hw/acpi.c >>>> +++ b/hw/acpi.c >>>> @@ -355,7 +355,7 @@ static uint8_t *acpi_gpe_ioport_get_ptr(ACPIGPE *gpe, uint32_t addr) >>>> if (addr < gpe->len / 2) { >>>> cur = gpe->sts + addr; >>>> } else if (addr < gpe->len) { >>>> - cur = gpe->en + addr; >>>> + cur = gpe->en + addr - gpe->len / 2; >>>> } else { >>>> abort(); >>>> } >>>> @@ -369,12 +369,8 @@ void acpi_gpe_ioport_writeb(ACPIGPE *gpe, uint32_t addr, uint32_t val) >>>> >>>> addr -= gpe->blk; >>>> cur = acpi_gpe_ioport_get_ptr(gpe, addr); >>>> - if (addr < gpe->len / 2) { >>>> - /* GPE_STS */ >>>> - *cur = (*cur) & ~val; >>>> - } else if (addr < gpe->len) { >>>> - /* GPE_EN */ >>>> - *cur = val; >>>> + if (cur != NULL) { >>>> + *cur = val & 0xff; >>>> } else { >>>> abort(); >>>> } >>>> -- >>>> 1.7.1 >>>> >> >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH V2] fix acpi regression 2011-04-12 5:13 [Qemu-devel] [PATCH] fix acpi regression Wen Congyang 2011-04-12 8:48 ` Isaku Yamahata @ 2011-04-12 9:27 ` Wen Congyang 2011-04-12 9:44 ` Isaku Yamahata 2011-04-12 21:35 ` Aurelien Jarno 1 sibling, 2 replies; 8+ messages in thread From: Wen Congyang @ 2011-04-12 9:27 UTC (permalink / raw) To: Isaku Yamahata, Aurelien Jarno, qemu-devel, blauwirbel This bug is introduced by commit 23910d3f. Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> --- hw/acpi.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/hw/acpi.c b/hw/acpi.c index e372474..ad40fb4 100644 --- a/hw/acpi.c +++ b/hw/acpi.c @@ -355,7 +355,7 @@ static uint8_t *acpi_gpe_ioport_get_ptr(ACPIGPE *gpe, uint32_t addr) if (addr < gpe->len / 2) { cur = gpe->sts + addr; } else if (addr < gpe->len) { - cur = gpe->en + addr; + cur = gpe->en + addr - gpe->len / 2; } else { abort(); } -- 1.7.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH V2] fix acpi regression 2011-04-12 9:27 ` [Qemu-devel] [PATCH V2] " Wen Congyang @ 2011-04-12 9:44 ` Isaku Yamahata 2011-04-12 21:35 ` Aurelien Jarno 1 sibling, 0 replies; 8+ messages in thread From: Isaku Yamahata @ 2011-04-12 9:44 UTC (permalink / raw) To: Wen Congyang; +Cc: blauwirbel, qemu-devel, Aurelien Jarno Thank you. Looks good. On Tue, Apr 12, 2011 at 05:27:44PM +0800, Wen Congyang wrote: > This bug is introduced by commit 23910d3f. > > Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> > > --- > hw/acpi.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/hw/acpi.c b/hw/acpi.c > index e372474..ad40fb4 100644 > --- a/hw/acpi.c > +++ b/hw/acpi.c > @@ -355,7 +355,7 @@ static uint8_t *acpi_gpe_ioport_get_ptr(ACPIGPE *gpe, uint32_t addr) > if (addr < gpe->len / 2) { > cur = gpe->sts + addr; > } else if (addr < gpe->len) { > - cur = gpe->en + addr; > + cur = gpe->en + addr - gpe->len / 2; > } else { > abort(); > } > -- > 1.7.1 > -- yamahata ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH V2] fix acpi regression 2011-04-12 9:27 ` [Qemu-devel] [PATCH V2] " Wen Congyang 2011-04-12 9:44 ` Isaku Yamahata @ 2011-04-12 21:35 ` Aurelien Jarno 1 sibling, 0 replies; 8+ messages in thread From: Aurelien Jarno @ 2011-04-12 21:35 UTC (permalink / raw) To: Wen Congyang; +Cc: blauwirbel, Isaku Yamahata, qemu-devel On Tue, Apr 12, 2011 at 05:27:44PM +0800, Wen Congyang wrote: > This bug is introduced by commit 23910d3f. > > Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> > > --- > hw/acpi.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) Thanks, applied. > diff --git a/hw/acpi.c b/hw/acpi.c > index e372474..ad40fb4 100644 > --- a/hw/acpi.c > +++ b/hw/acpi.c > @@ -355,7 +355,7 @@ static uint8_t *acpi_gpe_ioport_get_ptr(ACPIGPE *gpe, uint32_t addr) > if (addr < gpe->len / 2) { > cur = gpe->sts + addr; > } else if (addr < gpe->len) { > - cur = gpe->en + addr; > + cur = gpe->en + addr - gpe->len / 2; > } else { > abort(); > } > -- > 1.7.1 > > > -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2011-04-13 1:47 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-04-12 5:13 [Qemu-devel] [PATCH] fix acpi regression Wen Congyang 2011-04-12 8:48 ` Isaku Yamahata 2011-04-12 9:08 ` Wen Congyang 2011-04-12 9:20 ` Isaku Yamahata 2011-04-12 9:26 ` Wen Congyang 2011-04-12 9:27 ` [Qemu-devel] [PATCH V2] " Wen Congyang 2011-04-12 9:44 ` Isaku Yamahata 2011-04-12 21:35 ` Aurelien Jarno
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).