[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v6 29/30] hw/timer/sh_timer: Fix timer memory region size
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v6 29/30] hw/timer/sh_timer: Fix timer memory region size |
Date: |
Sat, 30 Oct 2021 11:41:47 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0 |
On 10/30/21 01:36, BALATON Zoltan wrote:
> On Sat, 30 Oct 2021, Philippe Mathieu-Daudé wrote:
>> On 10/29/21 23:02, BALATON Zoltan wrote:
>>> The timer memory region is only accessed via aliases that are 0x1000
>>> bytes long, no need to have the timer region larger than that.
>>>
>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>> ---
>>> hw/timer/sh_timer.c | 3 +--
>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/hw/timer/sh_timer.c b/hw/timer/sh_timer.c
>>> index 250ad41b48..a6445092e4 100644
>>> --- a/hw/timer/sh_timer.c
>>> +++ b/hw/timer/sh_timer.c
>>> @@ -350,8 +350,7 @@ void tmu012_init(MemoryRegion *sysmem, hwaddr
>>> base, int feat, uint32_t freq,
>>> ch2_irq0); /* ch2_irq1 not
>>> supported */
>>> }
>>>
>>> - memory_region_init_io(&s->iomem, NULL, &tmu012_ops, s,
>>> - "timer", 0x100000000ULL);
>>> + memory_region_init_io(&s->iomem, NULL, &tmu012_ops, s, "timer",
>>> 0x1000);
>>
>> Per the manual (R01UH0457EJ0401 Rev. 4.01 [*]) Page 317/1128, Table
>> 12.2 "TMU Registers" the first 3 timers (implemented by the tmu012_state
>> structure) fit in a region of 0x30 bytes.
>
> Sent a v7 of this patch only changing it to 0x30 with which the Linux
> image I've tested still boots but don't know if it uses a timer at all.
>
>> Looking at hw/timer/sh_timer.c I only see a maximum access of 0x40,
>
> Where do you see 0x40?
I just looked at tmu012_read(), not sh_timer_read(). Now looking at
it, the later only parses a 0x10 region, so total is 0x30 indeed.
>
>> where 0x1000 comes from? The P4/A7 aliases?
>
> Yes, as the commit message said. Since this was a last minute change I
> tried to be safe and not change anything guest visible at this point.
We should avoid last minute changes days before soft freeze ;)
>> If you have a way to test and ack, I can replace by 0x40 when applying.
>
> If you think 0x40 is better then I'm fine with that but I don't see a
> register after 0x2c which is 32 bits so 0x30 length should be enough
> according to that.
Yes, 0x30 is correct, I'll take your v7 patch.
Thanks,
Phil.
- Re: [PATCH v6 25/30] hw/intc/sh_intc: Simplify allocating sources array, (continued)
- [PATCH v6 26/30] hw/intc/sh_intc: Remove unneeded local variable initialisers, BALATON Zoltan, 2021/10/29
- [PATCH v6 27/30] hw/timer/sh_timer: Rename sh_timer_state to SHTimerState, BALATON Zoltan, 2021/10/29
- [PATCH v6 30/30] hw/timer/sh_timer: Remove use of hw_error, BALATON Zoltan, 2021/10/29
- [PATCH v6 29/30] hw/timer/sh_timer: Fix timer memory region size, BALATON Zoltan, 2021/10/29
- [PATCH v7 29/30] hw/timer/sh_timer: Fix timer memory region size, BALATON Zoltan, 2021/10/29
- [PATCH v6 21/30] hw/intc/sh_intc: Use array index instead of pointer arithmetics, BALATON Zoltan, 2021/10/29
- [PATCH v6 22/30] hw/intc/sh_intc: Inline and drop sh_intc_source() function, BALATON Zoltan, 2021/10/29
Re: [PATCH v6 00/30] More SH4 clean ups (including code style series), Philippe Mathieu-Daudé, 2021/10/29
Re: [PATCH v6 00/30] More SH4 clean ups (including code style series), Philippe Mathieu-Daudé, 2021/10/30