hw/misc/milkymist-pfpu: Fix pFPU region size

[PATCH-for-5.1] hw/misc/milkymist-pfpu: Fix pFPU region size

Posted by Philippe Mathieu-Daudé 5 years, 4 months ago

The last microcode word (address 0x6000.6ffc) is not reachable.
Correct the programmable FPU I/O size (which is 4 KiB) to be
able to use all the microcode area.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/misc/milkymist-pfpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/misc/milkymist-pfpu.c b/hw/misc/milkymist-pfpu.c
index 516825e83d..4fbe3e8971 100644
--- a/hw/misc/milkymist-pfpu.c
+++ b/hw/misc/milkymist-pfpu.c
@@ -507,7 +507,7 @@ static void milkymist_pfpu_realize(DeviceState *dev, Error **errp)
     sysbus_init_irq(sbd, &s->irq);
 
     memory_region_init_io(&s->regs_region, OBJECT(dev), &pfpu_mmio_ops, s,
-            "milkymist-pfpu", MICROCODE_END * 4);
+                          "milkymist-pfpu", 0x1000);
     sysbus_init_mmio(sbd, &s->regs_region);
 }
 
-- 
2.21.3

Re: [PATCH-for-5.1] hw/misc/milkymist-pfpu: Fix pFPU region size

Posted by Michael Walle 5 years, 3 months ago

Hi Philippe,

Am 2020-07-18 11:37, schrieb Philippe Mathieu-Daudé:
> The last microcode word (address 0x6000.6ffc) is not reachable.
> Correct the programmable FPU I/O size (which is 4 KiB) to be
> able to use all the microcode area.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/misc/milkymist-pfpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/misc/milkymist-pfpu.c b/hw/misc/milkymist-pfpu.c
> index 516825e83d..4fbe3e8971 100644
> --- a/hw/misc/milkymist-pfpu.c
> +++ b/hw/misc/milkymist-pfpu.c
> @@ -507,7 +507,7 @@ static void milkymist_pfpu_realize(DeviceState
> *dev, Error **errp)
>      sysbus_init_irq(sbd, &s->irq);
> 
>      memory_region_init_io(&s->regs_region, OBJECT(dev), 
> &pfpu_mmio_ops, s,
> -            "milkymist-pfpu", MICROCODE_END * 4);
> +                          "milkymist-pfpu", 0x1000);

Could you use one of the MICROCODE_ macros instead? maybe 
(MICROCODE_WORDS * 2)?

With that fixed:
Reviewed-by: Michael Walle <michael@walle.cc>

-michael

>      sysbus_init_mmio(sbd, &s->regs_region);
>  }

Re: [PATCH-for-5.1] hw/misc/milkymist-pfpu: Fix pFPU region size

Posted by Laurent Vivier 5 years, 2 months ago

Le 19/07/2020 à 19:23, Michael Walle a écrit :
> Hi Philippe,
> 
> Am 2020-07-18 11:37, schrieb Philippe Mathieu-Daudé:
>> The last microcode word (address 0x6000.6ffc) is not reachable.
>> Correct the programmable FPU I/O size (which is 4 KiB) to be
>> able to use all the microcode area.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  hw/misc/milkymist-pfpu.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/misc/milkymist-pfpu.c b/hw/misc/milkymist-pfpu.c
>> index 516825e83d..4fbe3e8971 100644
>> --- a/hw/misc/milkymist-pfpu.c
>> +++ b/hw/misc/milkymist-pfpu.c
>> @@ -507,7 +507,7 @@ static void milkymist_pfpu_realize(DeviceState
>> *dev, Error **errp)
>>      sysbus_init_irq(sbd, &s->irq);
>>
>>      memory_region_init_io(&s->regs_region, OBJECT(dev),
>> &pfpu_mmio_ops, s,
>> -            "milkymist-pfpu", MICROCODE_END * 4);
>> +                          "milkymist-pfpu", 0x1000);
> 
> Could you use one of the MICROCODE_ macros instead? maybe
> (MICROCODE_WORDS * 2)?
> 
> With that fixed:
> Reviewed-by: Michael Walle <michael@walle.cc>
> 

Philippe,

do you plan to repost with the change suggested by Michael?

Thanks,
Laurent

Re: [PATCH-for-5.1] hw/misc/milkymist-pfpu: Fix pFPU region size

Posted by Philippe Mathieu-Daudé 5 years, 2 months ago

On 9/1/20 8:57 AM, Laurent Vivier wrote:
> Le 19/07/2020 à 19:23, Michael Walle a écrit :
>> Hi Philippe,
>>
>> Am 2020-07-18 11:37, schrieb Philippe Mathieu-Daudé:
>>> The last microcode word (address 0x6000.6ffc) is not reachable.
>>> Correct the programmable FPU I/O size (which is 4 KiB) to be
>>> able to use all the microcode area.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>>  hw/misc/milkymist-pfpu.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/hw/misc/milkymist-pfpu.c b/hw/misc/milkymist-pfpu.c
>>> index 516825e83d..4fbe3e8971 100644
>>> --- a/hw/misc/milkymist-pfpu.c
>>> +++ b/hw/misc/milkymist-pfpu.c
>>> @@ -507,7 +507,7 @@ static void milkymist_pfpu_realize(DeviceState
>>> *dev, Error **errp)
>>>      sysbus_init_irq(sbd, &s->irq);
>>>
>>>      memory_region_init_io(&s->regs_region, OBJECT(dev),
>>> &pfpu_mmio_ops, s,
>>> -            "milkymist-pfpu", MICROCODE_END * 4);
>>> +                          "milkymist-pfpu", 0x1000);
>>
>> Could you use one of the MICROCODE_ macros instead? maybe
>> (MICROCODE_WORDS * 2)?
>>
>> With that fixed:
>> Reviewed-by: Michael Walle <michael@walle.cc>
>>
> 
> Philippe,
> 
> do you plan to repost with the change suggested by Michael?

Unfortunately I won't :( I spent 3h trying to do that, then found
myself rewriting half of the pfpu because this is not that easy, and
this code is now scheduled for deprecation. Not worth my time :(

> 
> Thanks,
> Laurent
>