Patchwork [1/2] m48t59: drop obsolete address base arithmetic

login
register
mail settings
Submitter Blue Swirl
Date Oct. 15, 2011, 1:50 p.m.
Message ID <CAAu8pHvdTudV1zKoVmX4hNDJw-Kjq5zcosnN3MnD9H3sGeYMdw@mail.gmail.com>
Download mbox | patch
Permalink /patch/119972/
State New
Headers show

Comments

Blue Swirl - Oct. 15, 2011, 1:50 p.m.
Remove now incorrect address base arithmetic, missed by
9936d6e42392f1440505dfa9df065eabd251cadf. Fixes Sparc64 boot.

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 hw/m48t59.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

     case 0:
@@ -505,7 +504,6 @@ static uint32_t NVRAM_readb (void *opaque, uint32_t addr)
     M48t59State *NVRAM = opaque;
     uint32_t retval;

-    addr -= NVRAM->io_base;
     switch (addr) {
     case 3:
         retval = m48t59_read(NVRAM, NVRAM->addr);
Andreas Färber - Jan. 5, 2012, 5:45 p.m.
Am 15.10.2011 15:50, schrieb Blue Swirl:
> Remove now incorrect address base arithmetic, missed by
> 9936d6e42392f1440505dfa9df065eabd251cadf. Fixes Sparc64 boot.

...but breaks PReP boot:

ERROR: BUG caught...
BIOS execution exception
nip=0x05800000 msr=0x00002000 dar=0x00000000 dsisr=0x00000000
Stopping execution

I verified by checking out the preceding commit, applying a variation of
http://patchwork.ozlabs.org/patch/134519/ on top; that restored PReP
boot to what it used to look like.

Any insights?

If I revert this commit on HEAD instead, then the above error
disappears, too, but there's another regression with the kernel not
being able to read the hda or something.

The write access fix seems unrelated.

Andreas

> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
> ---
>  hw/m48t59.c |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/m48t59.c b/hw/m48t59.c
> index f318e67..dba5796 100644
> --- a/hw/m48t59.c
> +++ b/hw/m48t59.c
> @@ -480,7 +480,6 @@ static void NVRAM_writeb (void *opaque, uint32_t
> addr, uint32_t val)
>  {
>      M48t59State *NVRAM = opaque;
> 
> -    addr -= NVRAM->io_base;
>      NVRAM_PRINTF("%s: 0x%08x => 0x%08x\n", __func__, addr, val);
>      switch (addr) {
>      case 0:
> @@ -505,7 +504,6 @@ static uint32_t NVRAM_readb (void *opaque, uint32_t addr)
>      M48t59State *NVRAM = opaque;
>      uint32_t retval;
> 
> -    addr -= NVRAM->io_base;
>      switch (addr) {
>      case 3:
>          retval = m48t59_read(NVRAM, NVRAM->addr);
Blue Swirl - Jan. 7, 2012, 5:29 p.m.
On Thu, Jan 5, 2012 at 17:45, Andreas Färber <afaerber@suse.de> wrote:
> Am 15.10.2011 15:50, schrieb Blue Swirl:
>> Remove now incorrect address base arithmetic, missed by
>> 9936d6e42392f1440505dfa9df065eabd251cadf. Fixes Sparc64 boot.
>
> ...but breaks PReP boot:
>
> ERROR: BUG caught...
> BIOS execution exception
> nip=0x05800000 msr=0x00002000 dar=0x00000000 dsisr=0x00000000
> Stopping execution
>
> I verified by checking out the preceding commit, applying a variation of
> http://patchwork.ozlabs.org/patch/134519/ on top; that restored PReP
> boot to what it used to look like.
>
> Any insights?

Sparc64 problem was that the io_base did not match what was passed to
the functions and then the calculation made the offset totally
incorrect. Could you add printfs to see what is the offset?

> If I revert this commit on HEAD instead, then the above error
> disappears, too, but there's another regression with the kernel not
> being able to read the hda or something.
>
> The write access fix seems unrelated.
>
> Andreas
>
>> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
>> ---
>>  hw/m48t59.c |    2 --
>>  1 files changed, 0 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/m48t59.c b/hw/m48t59.c
>> index f318e67..dba5796 100644
>> --- a/hw/m48t59.c
>> +++ b/hw/m48t59.c
>> @@ -480,7 +480,6 @@ static void NVRAM_writeb (void *opaque, uint32_t
>> addr, uint32_t val)
>>  {
>>      M48t59State *NVRAM = opaque;
>>
>> -    addr -= NVRAM->io_base;
>>      NVRAM_PRINTF("%s: 0x%08x => 0x%08x\n", __func__, addr, val);
>>      switch (addr) {
>>      case 0:
>> @@ -505,7 +504,6 @@ static uint32_t NVRAM_readb (void *opaque, uint32_t addr)
>>      M48t59State *NVRAM = opaque;
>>      uint32_t retval;
>>
>> -    addr -= NVRAM->io_base;
>>      switch (addr) {
>>      case 3:
>>          retval = m48t59_read(NVRAM, NVRAM->addr);
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Andreas Färber - Jan. 8, 2012, 2:48 a.m.
Am 07.01.2012 18:29, schrieb Blue Swirl:
> On Thu, Jan 5, 2012 at 17:45, Andreas Färber <afaerber@suse.de> wrote:
>> Am 15.10.2011 15:50, schrieb Blue Swirl:
>>> Remove now incorrect address base arithmetic, missed by
>>> 9936d6e42392f1440505dfa9df065eabd251cadf. Fixes Sparc64 boot.
>>
>> ...but breaks PReP boot:
>>
>> ERROR: BUG caught...
>> BIOS execution exception
>> nip=0x05800000 msr=0x00002000 dar=0x00000000 dsisr=0x00000000
>> Stopping execution
>>
>> I verified by checking out the preceding commit, applying a variation of
>> http://patchwork.ozlabs.org/patch/134519/ on top; that restored PReP
>> boot to what it used to look like.
>>
>> Any insights?
> 
> Sparc64 problem was that the io_base did not match what was passed to
> the functions and then the calculation made the offset totally
> incorrect. Could you add printfs to see what is the offset?

info qtree:

  dev: m48t59, id ""
    dev-prop: size = 8192
    dev-prop: type = 59
    dev-prop: io_base = 0x74
    irq 1
    mmio ffffffffffffffff/0000000000002000

#define DEBUG_NVRAM:

NVRAM_writeb: 0x00000074 => 0x00000000
NVRAM_writeb: 0x00000075 => 0x00000000
NVRAM_readb: 0x00000077 <= 0xffffffff
NVRAM_writeb: 0x00000074 => 0x00000001
NVRAM_writeb: 0x00000075 => 0x00000000
NVRAM_readb: 0x00000077 <= 0xffffffff
NVRAM_writeb: 0x00000074 => 0x00000002
NVRAM_writeb: 0x00000075 => 0x00000000
NVRAM_readb: 0x00000077 <= 0xffffffff
NVRAM_writeb: 0x00000074 => 0x00000003
NVRAM_writeb: 0x00000075 => 0x00000000
NVRAM_readb: 0x00000077 <= 0xffffffff
NVRAM_writeb: 0x00000074 => 0x00000004
NVRAM_writeb: 0x00000075 => 0x00000000
NVRAM_readb: 0x00000077 <= 0xffffffff
NVRAM_writeb: 0x00000074 => 0x00000005
NVRAM_writeb: 0x00000075 => 0x00000000
NVRAM_readb: 0x00000077 <= 0xffffffff
NVRAM_writeb: 0x00000074 => 0x00000006
NVRAM_writeb: 0x00000075 => 0x00000000
NVRAM_readb: 0x00000077 <= 0xffffffff
NVRAM_writeb: 0x00000074 => 0x00000007
NVRAM_writeb: 0x00000075 => 0x00000000
NVRAM_readb: 0x00000077 <= 0xffffffff
NVRAM_writeb: 0x00000074 => 0x00000008
NVRAM_writeb: 0x00000075 => 0x00000000
NVRAM_readb: 0x00000077 <= 0xffffffff
NVRAM_writeb: 0x00000074 => 0x00000009
NVRAM_writeb: 0x00000075 => 0x00000000
NVRAM_readb: 0x00000077 <= 0xffffffff
NVRAM_writeb: 0x00000074 => 0x0000000a
NVRAM_writeb: 0x00000075 => 0x00000000
NVRAM_readb: 0x00000077 <= 0xffffffff
NVRAM_writeb: 0x00000074 => 0x0000000b
NVRAM_writeb: 0x00000075 => 0x00000000
NVRAM_readb: 0x00000077 <= 0xffffffff
NVRAM_writeb: 0x00000074 => 0x0000000c
NVRAM_writeb: 0x00000075 => 0x00000000
NVRAM_readb: 0x00000077 <= 0xffffffff
NVRAM_writeb: 0x00000074 => 0x0000000d
NVRAM_writeb: 0x00000075 => 0x00000000
NVRAM_readb: 0x00000077 <= 0xffffffff
NVRAM_writeb: 0x00000074 => 0x0000000e
NVRAM_writeb: 0x00000075 => 0x00000000
NVRAM_readb: 0x00000077 <= 0xffffffff
NVRAM_writeb: 0x00000074 => 0x0000000f
NVRAM_writeb: 0x00000075 => 0x00000000
NVRAM_readb: 0x00000077 <= 0xffffffff

Andreas
Blue Swirl - Jan. 8, 2012, 11:43 a.m.
On Sun, Jan 8, 2012 at 02:48, Andreas Färber <afaerber@suse.de> wrote:
> Am 07.01.2012 18:29, schrieb Blue Swirl:
>> On Thu, Jan 5, 2012 at 17:45, Andreas Färber <afaerber@suse.de> wrote:
>>> Am 15.10.2011 15:50, schrieb Blue Swirl:
>>>> Remove now incorrect address base arithmetic, missed by
>>>> 9936d6e42392f1440505dfa9df065eabd251cadf. Fixes Sparc64 boot.
>>>
>>> ...but breaks PReP boot:
>>>
>>> ERROR: BUG caught...
>>> BIOS execution exception
>>> nip=0x05800000 msr=0x00002000 dar=0x00000000 dsisr=0x00000000
>>> Stopping execution
>>>
>>> I verified by checking out the preceding commit, applying a variation of
>>> http://patchwork.ozlabs.org/patch/134519/ on top; that restored PReP
>>> boot to what it used to look like.
>>>
>>> Any insights?
>>
>> Sparc64 problem was that the io_base did not match what was passed to
>> the functions and then the calculation made the offset totally
>> incorrect. Could you add printfs to see what is the offset?
>
> info qtree:
>
>  dev: m48t59, id ""
>    dev-prop: size = 8192
>    dev-prop: type = 59
>    dev-prop: io_base = 0x74
>    irq 1
>    mmio ffffffffffffffff/0000000000002000
>
> #define DEBUG_NVRAM:
>
> NVRAM_writeb: 0x00000074 => 0x00000000
> NVRAM_writeb: 0x00000075 => 0x00000000
> NVRAM_readb: 0x00000077 <= 0xffffffff
> NVRAM_writeb: 0x00000074 => 0x00000001
> NVRAM_writeb: 0x00000075 => 0x00000000
> NVRAM_readb: 0x00000077 <= 0xffffffff
> NVRAM_writeb: 0x00000074 => 0x00000002
> NVRAM_writeb: 0x00000075 => 0x00000000
> NVRAM_readb: 0x00000077 <= 0xffffffff
> NVRAM_writeb: 0x00000074 => 0x00000003
> NVRAM_writeb: 0x00000075 => 0x00000000
> NVRAM_readb: 0x00000077 <= 0xffffffff
> NVRAM_writeb: 0x00000074 => 0x00000004
> NVRAM_writeb: 0x00000075 => 0x00000000
> NVRAM_readb: 0x00000077 <= 0xffffffff
> NVRAM_writeb: 0x00000074 => 0x00000005
> NVRAM_writeb: 0x00000075 => 0x00000000
> NVRAM_readb: 0x00000077 <= 0xffffffff
> NVRAM_writeb: 0x00000074 => 0x00000006
> NVRAM_writeb: 0x00000075 => 0x00000000
> NVRAM_readb: 0x00000077 <= 0xffffffff
> NVRAM_writeb: 0x00000074 => 0x00000007
> NVRAM_writeb: 0x00000075 => 0x00000000
> NVRAM_readb: 0x00000077 <= 0xffffffff
> NVRAM_writeb: 0x00000074 => 0x00000008
> NVRAM_writeb: 0x00000075 => 0x00000000
> NVRAM_readb: 0x00000077 <= 0xffffffff
> NVRAM_writeb: 0x00000074 => 0x00000009
> NVRAM_writeb: 0x00000075 => 0x00000000
> NVRAM_readb: 0x00000077 <= 0xffffffff
> NVRAM_writeb: 0x00000074 => 0x0000000a
> NVRAM_writeb: 0x00000075 => 0x00000000
> NVRAM_readb: 0x00000077 <= 0xffffffff
> NVRAM_writeb: 0x00000074 => 0x0000000b
> NVRAM_writeb: 0x00000075 => 0x00000000
> NVRAM_readb: 0x00000077 <= 0xffffffff
> NVRAM_writeb: 0x00000074 => 0x0000000c
> NVRAM_writeb: 0x00000075 => 0x00000000
> NVRAM_readb: 0x00000077 <= 0xffffffff
> NVRAM_writeb: 0x00000074 => 0x0000000d
> NVRAM_writeb: 0x00000075 => 0x00000000
> NVRAM_readb: 0x00000077 <= 0xffffffff
> NVRAM_writeb: 0x00000074 => 0x0000000e
> NVRAM_writeb: 0x00000075 => 0x00000000
> NVRAM_readb: 0x00000077 <= 0xffffffff
> NVRAM_writeb: 0x00000074 => 0x0000000f
> NVRAM_writeb: 0x00000075 => 0x00000000
> NVRAM_readb: 0x00000077 <= 0xffffffff

This is what happens on Sparc64:
NVRAM_writeb: 0x00000000 => 0x00000000
NVRAM_writeb: 0x00000001 => 0x00000000
NVRAM_readb: 0x00000003 <= 0x00000070
NVRAM_writeb: 0x00000000 => 0x00000001
NVRAM_writeb: 0x00000001 => 0x00000000
NVRAM_readb: 0x00000003 <= 0x0000001a
NVRAM_writeb: 0x00000000 => 0x00000002
NVRAM_writeb: 0x00000001 => 0x00000000
NVRAM_readb: 0x00000003 <= 0x00000000
NVRAM_writeb: 0x00000000 => 0x00000003
NVRAM_writeb: 0x00000001 => 0x00000000
NVRAM_readb: 0x00000003 <= 0x00000002
NVRAM_writeb: 0x00000000 => 0x00000004
NVRAM_writeb: 0x00000001 => 0x00000000
NVRAM_readb: 0x00000003 <= 0x00000073
NVRAM_writeb: 0x00000000 => 0x00000005
NVRAM_writeb: 0x00000001 => 0x00000000
NVRAM_readb: 0x00000003 <= 0x00000079
NVRAM_writeb: 0x00000000 => 0x00000006
NVRAM_writeb: 0x00000001 => 0x00000000
NVRAM_readb: 0x00000003 <= 0x00000073
NVRAM_writeb: 0x00000000 => 0x00000007
NVRAM_writeb: 0x00000001 => 0x00000000

I/O base offset is kept with PIO but removed from MMIO, which is not
consistent. Avi, could we fix this?

Alternatively there could be different handlers for MMIO and PIO, or
in this case masking the address with 0x3 could be used but I'd rather
unify the I/O handlers.
Avi Kivity - Jan. 8, 2012, 12:16 p.m.
On 01/08/2012 01:43 PM, Blue Swirl wrote:
> On Sun, Jan 8, 2012 at 02:48, Andreas Färber <afaerber@suse.de> wrote:
> > Am 07.01.2012 18:29, schrieb Blue Swirl:
> >> On Thu, Jan 5, 2012 at 17:45, Andreas Färber <afaerber@suse.de> wrote:
> >>> Am 15.10.2011 15:50, schrieb Blue Swirl:
> >>>> Remove now incorrect address base arithmetic, missed by
> >>>> 9936d6e42392f1440505dfa9df065eabd251cadf. Fixes Sparc64 boot.
> >>>
> >>> ...but breaks PReP boot:
> >>>
> >>> ERROR: BUG caught...
> >>> BIOS execution exception
> >>> nip=0x05800000 msr=0x00002000 dar=0x00000000 dsisr=0x00000000
> >>> Stopping execution
> >>>
> >>> I verified by checking out the preceding commit, applying a variation of
> >>> http://patchwork.ozlabs.org/patch/134519/ on top; that restored PReP
> >>> boot to what it used to look like.
> >>>
> >>> Any insights?
> >>
> >> Sparc64 problem was that the io_base did not match what was passed to
> >> the functions and then the calculation made the offset totally
> >> incorrect. Could you add printfs to see what is the offset?
> >
> > info qtree:
> >
> >  dev: m48t59, id ""
> >    dev-prop: size = 8192
> >    dev-prop: type = 59
> >    dev-prop: io_base = 0x74
> >    irq 1
> >    mmio ffffffffffffffff/0000000000002000
> >
> > #define DEBUG_NVRAM:
> >
> > NVRAM_writeb: 0x00000074 => 0x00000000
> > NVRAM_writeb: 0x00000075 => 0x00000000
> > NVRAM_readb: 0x00000077 <= 0xffffffff
> > NVRAM_writeb: 0x00000074 => 0x00000001
> > NVRAM_writeb: 0x00000075 => 0x00000000
> > NVRAM_readb: 0x00000077 <= 0xffffffff
> > NVRAM_writeb: 0x00000074 => 0x00000002
> > NVRAM_writeb: 0x00000075 => 0x00000000
> > NVRAM_readb: 0x00000077 <= 0xffffffff
> > NVRAM_writeb: 0x00000074 => 0x00000003
> > NVRAM_writeb: 0x00000075 => 0x00000000
> > NVRAM_readb: 0x00000077 <= 0xffffffff
> > NVRAM_writeb: 0x00000074 => 0x00000004
> > NVRAM_writeb: 0x00000075 => 0x00000000
> > NVRAM_readb: 0x00000077 <= 0xffffffff
> > NVRAM_writeb: 0x00000074 => 0x00000005
> > NVRAM_writeb: 0x00000075 => 0x00000000
> > NVRAM_readb: 0x00000077 <= 0xffffffff
> > NVRAM_writeb: 0x00000074 => 0x00000006
> > NVRAM_writeb: 0x00000075 => 0x00000000
> > NVRAM_readb: 0x00000077 <= 0xffffffff
> > NVRAM_writeb: 0x00000074 => 0x00000007
> > NVRAM_writeb: 0x00000075 => 0x00000000
> > NVRAM_readb: 0x00000077 <= 0xffffffff
> > NVRAM_writeb: 0x00000074 => 0x00000008
> > NVRAM_writeb: 0x00000075 => 0x00000000
> > NVRAM_readb: 0x00000077 <= 0xffffffff
> > NVRAM_writeb: 0x00000074 => 0x00000009
> > NVRAM_writeb: 0x00000075 => 0x00000000
> > NVRAM_readb: 0x00000077 <= 0xffffffff
> > NVRAM_writeb: 0x00000074 => 0x0000000a
> > NVRAM_writeb: 0x00000075 => 0x00000000
> > NVRAM_readb: 0x00000077 <= 0xffffffff
> > NVRAM_writeb: 0x00000074 => 0x0000000b
> > NVRAM_writeb: 0x00000075 => 0x00000000
> > NVRAM_readb: 0x00000077 <= 0xffffffff
> > NVRAM_writeb: 0x00000074 => 0x0000000c
> > NVRAM_writeb: 0x00000075 => 0x00000000
> > NVRAM_readb: 0x00000077 <= 0xffffffff
> > NVRAM_writeb: 0x00000074 => 0x0000000d
> > NVRAM_writeb: 0x00000075 => 0x00000000
> > NVRAM_readb: 0x00000077 <= 0xffffffff
> > NVRAM_writeb: 0x00000074 => 0x0000000e
> > NVRAM_writeb: 0x00000075 => 0x00000000
> > NVRAM_readb: 0x00000077 <= 0xffffffff
> > NVRAM_writeb: 0x00000074 => 0x0000000f
> > NVRAM_writeb: 0x00000075 => 0x00000000
> > NVRAM_readb: 0x00000077 <= 0xffffffff
>
> This is what happens on Sparc64:
> NVRAM_writeb: 0x00000000 => 0x00000000
> NVRAM_writeb: 0x00000001 => 0x00000000
> NVRAM_readb: 0x00000003 <= 0x00000070
> NVRAM_writeb: 0x00000000 => 0x00000001
> NVRAM_writeb: 0x00000001 => 0x00000000
> NVRAM_readb: 0x00000003 <= 0x0000001a
> NVRAM_writeb: 0x00000000 => 0x00000002
> NVRAM_writeb: 0x00000001 => 0x00000000
> NVRAM_readb: 0x00000003 <= 0x00000000
> NVRAM_writeb: 0x00000000 => 0x00000003
> NVRAM_writeb: 0x00000001 => 0x00000000
> NVRAM_readb: 0x00000003 <= 0x00000002
> NVRAM_writeb: 0x00000000 => 0x00000004
> NVRAM_writeb: 0x00000001 => 0x00000000
> NVRAM_readb: 0x00000003 <= 0x00000073
> NVRAM_writeb: 0x00000000 => 0x00000005
> NVRAM_writeb: 0x00000001 => 0x00000000
> NVRAM_readb: 0x00000003 <= 0x00000079
> NVRAM_writeb: 0x00000000 => 0x00000006
> NVRAM_writeb: 0x00000001 => 0x00000000
> NVRAM_readb: 0x00000003 <= 0x00000073
> NVRAM_writeb: 0x00000000 => 0x00000007
> NVRAM_writeb: 0x00000001 => 0x00000000
>
> I/O base offset is kept with PIO but removed from MMIO, which is not
> consistent. Avi, could we fix this?

Yes, I'll work on it.

Patch

diff --git a/hw/m48t59.c b/hw/m48t59.c
index f318e67..dba5796 100644
--- a/hw/m48t59.c
+++ b/hw/m48t59.c
@@ -480,7 +480,6 @@  static void NVRAM_writeb (void *opaque, uint32_t
addr, uint32_t val)
 {
     M48t59State *NVRAM = opaque;

-    addr -= NVRAM->io_base;
     NVRAM_PRINTF("%s: 0x%08x => 0x%08x\n", __func__, addr, val);
     switch (addr) {