Re: [Qemu-devel] Re: [PATCH] sparc32: ledma extra registers

[Artyom Tarasenko <atar4qemu@gmail.com>]

On Mon, Dec 20, 2010 at 6:33 PM, Bob Breuer <breuerr@mc.net> wrote:
> Artyom Tarasenko wrote:
>> On Sun, Dec 19, 2010 at 8:37 PM, Bob Breuer <breuerr@mc.net> wrote:
>>
>>> Andreas Färber wrote:
>>>
>>>> Am 18.12.2010 um 19:53 schrieb Blue Swirl:
>>>>
>>>>
>>>>> On Sat, Dec 18, 2010 at 5:09 PM, Bob Breuer <breuerr@mc.net> wrote:
>>>>>
>>>>>> ledma has 0x20 bytes of registers according to OBP, and at least
>>>>>> Solaris9
>>>>>> reads the 5th register which is beyond what we've mapped.  So let's
>>>>>> setup
>>>>>> a flag (inspired by a previous patch from Blue Swirl) to identify ledma
>>>>>> from espdma, and map another 16 bytes of registers which return 0.
>>>>>>
>>>>>> Signed-off-by: Bob Breuer <breuerr@mc.net>
>>>>>>
>>>> I'm not familar with that part of code but...
>>>>
>>>>
>>>>>> diff --git a/hw/sparc32_dma.c b/hw/sparc32_dma.c
>>>>>> index e78f025..56be8c8 100644
>>>>>> --- a/hw/sparc32_dma.c
>>>>>> +++ b/hw/sparc32_dma.c
>>>>>>
>>>>>> @@ -165,6 +169,9 @@ static uint32_t dma_mem_readl(void *opaque,
>>>>>> target_phys_addr_t addr)
>>>>>>     DMAState *s = opaque;
>>>>>>     uint32_t saddr;
>>>>>>
>>>>>> +    if (s->is_ledma && (addr > DMA_MAX_REG_OFFSET)) {
>>>>>> +        return 0; /* extra mystery register(s) */
>>>>>>
>>>> Wouldn't it be a good idea to trace these "mystery" reads...
>>>>
>>>>
>>>>>> +    }
>>>>>>     saddr = (addr & DMA_MASK) >> 2;
>>>>>>     trace_sparc32_dma_mem_readl(addr, s->dmaregs[saddr]);
>>>>>>     return s->dmaregs[saddr];
>>>>>> @@ -175,6 +182,9 @@ static void dma_mem_writel(void *opaque,
>>>>>> target_phys_addr_t addr, uint32_t val)
>>>>>>     DMAState *s = opaque;
>>>>>>     uint32_t saddr;
>>>>>>
>>>>>> +    if (s->is_ledma && (addr > DMA_MAX_REG_OFFSET)) {
>>>>>> +        return; /* extra mystery register(s) */
>>>>>>
>>>> ...and writes? We return just before the tracepoints fire.
>>>>
>>>>
>>> Ok, I'll put together a patch to add the trace calls just before the
>>> returns.  How about I also call it undocumented instead of mystery.
>>> None of the BSD's or Linux know about or use anything beyond the 4
>>> registers.
>>>
>>
>> I'd use "aliased" instead of mystery.  On a real SS-5:
>>
>> ok 78400020 20 spacel@ .
>> a4240050
>> ok 78400000 20 spacel@ .
>> a4240050
>> ok 78400024 20 spacel@ .
>> fc004000
>> ok 78400004 20 spacel@ .
>> fc004000
>>
> Verified that it also aliases on an SS-20.
>> Addresses 0x7840002x are aliases for 0x7840000x. As well as
>> 0x7840004x. And so on up to
>> ok 787fffe4 20 spacel@ .
>> fc004000
>> 78800004 20 spacel@ .
>> 0
>>
>> Or a real SS-20 ef0400000 is aliased up to ef37fffe0
>>
>> Fwiw I think it's a bug in the later Solaris versions:
>> http://tyom.blogspot.com/2010/10/bug-in-all-solaris-versions-after-57.html
>>
>> On the bare metal it works because of address aliasing. If you want to
>> emulate the hw precisely, the Blue's generic aliasing patch can be
>> used here. The question is though do we want to do a generic aliasing
>> for all the SBUS devices, or just in the single case(es) where we know
>> is necessary.
>>
>> On the other hand Solaris seems to be fine with a 0 stub too.
>>
> I'll send a patch to update the comments.  If it's accessing a wrong
> register because of a bug, then it may not matter what value is returned.

I think the value is important, but the tests show that 0 is fine.


-- 
Regards,
Artyom Tarasenko

solaris/sparc under qemu blog: http://tyom.blogspot.com/