Patchwork arm: vexpress: Clear sysctl cfgctrl start bit

login
register
mail settings
Submitter Christoffer Dall
Date Dec. 22, 2012, 7:03 p.m.
Message ID <1356202993-55730-1-git-send-email-c.dall@virtualopensystems.com>
Download mbox | patch
Permalink /patch/207935/
State New
Headers show

Comments

Christoffer Dall - Dec. 22, 2012, 7:03 p.m.
The start bit should only be set to indicate that a function call is
underway, right now.  When done with function, clear it.

Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Christoffer Dall <c.dall@virtualopensystems.com>
---
 hw/arm_sysctl.c |    1 +
 1 file changed, 1 insertion(+)
Peter Maydell - Dec. 23, 2012, 11:01 a.m.
On 22 December 2012 19:03, Christoffer Dall
<c.dall@virtualopensystems.com> wrote:
> The start bit should only be set to indicate that a function call is
> underway, right now.  When done with function, clear it.

Looks plausible but I'd like a crosscheck that this is how the hardware
really behaves.

-- PMM
Christoffer Dall - Dec. 23, 2012, 4:34 p.m.
On Sun, Dec 23, 2012 at 6:01 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 22 December 2012 19:03, Christoffer Dall
> <c.dall@virtualopensystems.com> wrote:
>> The start bit should only be set to indicate that a function call is
>> underway, right now.  When done with function, clear it.
>
> Looks plausible but I'd like a crosscheck that this is how the hardware
> really behaves.
>
sure, on my vexpress box that's certainly the case by experimentation though.

-Christoffer
Peter Maydell - Jan. 24, 2013, 11:17 a.m.
On 23 December 2012 16:34, Christoffer Dall
<c.dall@virtualopensystems.com> wrote:
> On Sun, Dec 23, 2012 at 6:01 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 22 December 2012 19:03, Christoffer Dall
>> <c.dall@virtualopensystems.com> wrote:
>>> The start bit should only be set to indicate that a function call is
>>> underway, right now.  When done with function, clear it.
>>
>> Looks plausible but I'd like a crosscheck that this is how the hardware
>> really behaves.
>>
> sure, on my vexpress box that's certainly the case by experimentation though.

OK, if you've compared with hardware I'm happy.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM
Christoffer Dall - Jan. 24, 2013, 3:43 p.m.
On Thu, Jan 24, 2013 at 6:17 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 23 December 2012 16:34, Christoffer Dall
> <c.dall@virtualopensystems.com> wrote:
>> On Sun, Dec 23, 2012 at 6:01 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> On 22 December 2012 19:03, Christoffer Dall
>>> <c.dall@virtualopensystems.com> wrote:
>>>> The start bit should only be set to indicate that a function call is
>>>> underway, right now.  When done with function, clear it.
>>>
>>> Looks plausible but I'd like a crosscheck that this is how the hardware
>>> really behaves.
>>>
>> sure, on my vexpress box that's certainly the case by experimentation though.
>
> OK, if you've compared with hardware I'm happy.
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
Will you include this with other patches for upstream, or?
Peter Maydell - Jan. 24, 2013, 3:50 p.m.
On 24 January 2013 15:43, Christoffer Dall
<c.dall@virtualopensystems.com> wrote:
> On Thu, Jan 24, 2013 at 6:17 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> OK, if you've compared with hardware I'm happy.
>>
>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>>
> Will you include this with other patches for upstream, or?

Yes, I'll put it in my arm-devs.next tree.

-- PMM
Alexander Graf - Jan. 24, 2013, 6:54 p.m.
On 22.12.2012, at 20:03, Christoffer Dall wrote:

> The start bit should only be set to indicate that a function call is
> underway, right now.  When done with function, clear it.

Is this the reason 3.8 doesn't run properly?


Alex

> 
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Christoffer Dall <c.dall@virtualopensystems.com>
> ---
> hw/arm_sysctl.c |    1 +
> 1 file changed, 1 insertion(+)
> 
> diff --git a/hw/arm_sysctl.c b/hw/arm_sysctl.c
> index 58eb982..12a62fe 100644
> --- a/hw/arm_sysctl.c
> +++ b/hw/arm_sysctl.c
> @@ -332,6 +332,7 @@ static void arm_sysctl_write(void *opaque, hwaddr offset,
>         default:
>             s->sys_cfgstat |= 2;        /* error */
>         }
> +        s->sys_cfgctrl &= ~(1 << 31);
>         return;
>     case 0xa8: /* SYS_CFGSTAT */
>         if (board_id(s) != BOARD_ID_VEXPRESS) {
> -- 
> 1.7.9.5
> 
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
Peter Maydell - Jan. 24, 2013, 6:57 p.m.
On 24 January 2013 18:54, Alexander Graf <agraf@suse.de> wrote:
> On 22.12.2012, at 20:03, Christoffer Dall wrote:
>> The start bit should only be set to indicate that a function call is
>> underway, right now.  When done with function, clear it.
>
> Is this the reason 3.8 doesn't run properly?

It appears to be one of the reasons. The other is that
the kernel is trashing the device tree because it
happens to be sitting in the same page as the last
part-page of the initrd, and free_initrd_mem() rounds
up to a page boundary when poisoning memory after
the initrd has been uncompressed.

I think there is also some noise because we don't
emulate the oscillator and voltage regulator control
sysregs, but the kernel boots anyway.

-- PMM
Alexander Graf - Jan. 24, 2013, 7:01 p.m.
On 24.01.2013, at 19:57, Peter Maydell wrote:

> On 24 January 2013 18:54, Alexander Graf <agraf@suse.de> wrote:
>> On 22.12.2012, at 20:03, Christoffer Dall wrote:
>>> The start bit should only be set to indicate that a function call is
>>> underway, right now.  When done with function, clear it.
>> 
>> Is this the reason 3.8 doesn't run properly?
> 
> It appears to be one of the reasons. The other is that
> the kernel is trashing the device tree because it
> happens to be sitting in the same page as the last
> part-page of the initrd, and free_initrd_mem() rounds
> up to a page boundary when poisoning memory after
> the initrd has been uncompressed.
> 
> I think there is also some noise because we don't
> emulate the oscillator and voltage regulator control
> sysregs, but the kernel boots anyway.

Does it reboot / shut down for you with this patch?


Alex
Alexander Spyridakis - Jan. 24, 2013, 7:03 p.m.
On 24 January 2013 19:57, Peter Maydell <peter.maydell@linaro.org> wrote:

> It appears to be one of the reasons. The other is that
> the kernel is trashing the device tree because it
> happens to be sitting in the same page as the last
> part-page of the initrd, and free_initrd_mem() rounds
> up to a page boundary when poisoning memory after
> the initrd has been uncompressed.
>

I don't know if this is related, but in the past I remember QEMU behaving
quite differently when the DTB was appended to the kernel rather than
provided in the QEMU arguments (The latter would crash contrary to the
first).
Peter Maydell - Jan. 24, 2013, 7:16 p.m.
On 24 January 2013 19:01, Alexander Graf <agraf@suse.de> wrote:
> Does it reboot / shut down for you with this patch?

Well, my test 3.8-rc4 kernel for vexpress-a9 reboots
and shuts down ok, yes.

-- PMM
Alexander Graf - Jan. 24, 2013, 7:21 p.m.
On 24.01.2013, at 20:16, Peter Maydell wrote:

> On 24 January 2013 19:01, Alexander Graf <agraf@suse.de> wrote:
>> Does it reboot / shut down for you with this patch?
> 
> Well, my test 3.8-rc4 kernel for vexpress-a9 reboots
> and shuts down ok, yes.

Awesome. I tried vexpress-a15 with kvm this week and there my rc3 based kernel complained about sysctl not responding correctly on shut down. So if this patch is the one fixing that, it would be nice to have the patch in 1.4.


Alex

Patch

diff --git a/hw/arm_sysctl.c b/hw/arm_sysctl.c
index 58eb982..12a62fe 100644
--- a/hw/arm_sysctl.c
+++ b/hw/arm_sysctl.c
@@ -332,6 +332,7 @@  static void arm_sysctl_write(void *opaque, hwaddr offset,
         default:
             s->sys_cfgstat |= 2;        /* error */
         }
+        s->sys_cfgctrl &= ~(1 << 31);
         return;
     case 0xa8: /* SYS_CFGSTAT */
         if (board_id(s) != BOARD_ID_VEXPRESS) {