Re: [PATCH v3] Add basic power management to raspi.

2021-06-29 Thread Nolan

On 6/29/21 12:46 AM, Philippe Mathieu-Daudé wrote:

I had to do move bcm2835_peripherals.c to build (otherwise meson
complains and refuses to finish the configure script). I assumed it was
a problem on my side (or with my git version) and didn't noticed
bcm2835_peripherals.h was not under include/.


This must have been an artifact of my busted diff, in my tree:
$ find . -name bcm2835_peripherals.h
./include/hw/arm/bcm2835_peripherals.h


Nolan, can you tell us what OS/distribution you are using? You used
git v2.30.2 which might be badly packaged there, and could deserve
a proper bug report.


Debian Bullseye. I had diff.noprefix=true set in my .gitconfig for some 
reason or another. I've removed it.




[PATCH v3] Add basic power management to raspi.

2021-06-25 Thread Nolan Leake
This is just enough to make reboot and poweroff work. Works for
linux, u-boot, and the arm trusted firmware. Not tested, but should
work for plan9, and bare-metal/hobby OSes, since they seem to generally
do what linux does for reset.

The watchdog timer functionality is not yet implemented.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/64
Signed-off-by: Nolan Leake 
---
 hw/arm/bcm2835_peripherals.c |  13 ++-
 hw/misc/bcm2835_powermgt.c   | 160 +++
 hw/misc/meson.build  |   1 +
 include/hw/arm/bcm2835_peripherals.h |   3 +-
 include/hw/misc/bcm2835_powermgt.h   |  29 +
 5 files changed, 204 insertions(+), 2 deletions(-)
 create mode 100644 hw/misc/bcm2835_powermgt.c
 create mode 100644 include/hw/misc/bcm2835_powermgt.h

diff --git hw/arm/bcm2835_peripherals.c hw/arm/bcm2835_peripherals.c
index dcff13433e..48538c9360 100644
--- hw/arm/bcm2835_peripherals.c
+++ hw/arm/bcm2835_peripherals.c
@@ -126,6 +126,10 @@ static void bcm2835_peripherals_init(Object *obj)
 
 object_property_add_const_link(OBJECT(>dwc2), "dma-mr",
OBJECT(>gpu_bus_mr));
+
+/* Power Management */
+object_initialize_child(obj, "powermgt", >powermgt,
+TYPE_BCM2835_POWERMGT);
 }
 
 static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
@@ -364,9 +368,16 @@ static void bcm2835_peripherals_realize(DeviceState *dev, 
Error **errp)
 qdev_get_gpio_in_named(DEVICE(>ic), BCM2835_IC_GPU_IRQ,
INTERRUPT_USB));
 
+/* Power Management */
+if (!sysbus_realize(SYS_BUS_DEVICE(>powermgt), errp)) {
+return;
+}
+
+memory_region_add_subregion(>peri_mr, PM_OFFSET,
+sysbus_mmio_get_region(SYS_BUS_DEVICE(>powermgt), 0));
+
 create_unimp(s, >txp, "bcm2835-txp", TXP_OFFSET, 0x1000);
 create_unimp(s, >armtmr, "bcm2835-sp804", ARMCTRL_TIMER0_1_OFFSET, 
0x40);
-create_unimp(s, >powermgt, "bcm2835-powermgt", PM_OFFSET, 0x114);
 create_unimp(s, >i2s, "bcm2835-i2s", I2S_OFFSET, 0x100);
 create_unimp(s, >smi, "bcm2835-smi", SMI_OFFSET, 0x100);
 create_unimp(s, >spi[0], "bcm2835-spi0", SPI0_OFFSET, 0x20);
diff --git hw/misc/bcm2835_powermgt.c hw/misc/bcm2835_powermgt.c
new file mode 100644
index 00..dcdd6d1ea7
--- /dev/null
+++ hw/misc/bcm2835_powermgt.c
@@ -0,0 +1,160 @@
+/*
+ * BCM2835 Power Management emulation
+ *
+ * Copyright (C) 2017 Marcin Chojnacki 
+ * Copyright (C) 2021 Nolan Leake 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "hw/misc/bcm2835_powermgt.h"
+#include "migration/vmstate.h"
+#include "sysemu/runstate.h"
+
+#define PASSWORD 0x5a00
+#define PASSWORD_MASK 0xff00
+
+#define R_RSTC 0x1c
+#define V_RSTC_RESET 0x20
+#define R_RSTS 0x20
+#define V_RSTS_POWEROFF 0x555 /* Linux uses partition 63 to indicate halt. */
+#define R_WDOG 0x24
+
+static uint64_t bcm2835_powermgt_read(void *opaque, hwaddr offset,
+  unsigned size)
+{
+BCM2835PowerMgtState *s = (BCM2835PowerMgtState *)opaque;
+uint32_t res = 0;
+
+switch (offset) {
+case R_RSTC:
+res = s->rstc;
+break;
+case R_RSTS:
+res = s->rsts;
+break;
+case R_WDOG:
+res = s->wdog;
+break;
+
+default:
+qemu_log_mask(LOG_UNIMP,
+  "bcm2835_powermgt_read: Unknown offset 0x%08"HWADDR_PRIx
+  "\n", offset);
+res = 0;
+break;
+}
+
+return res;
+}
+
+static void bcm2835_powermgt_write(void *opaque, hwaddr offset,
+   uint64_t value, unsigned size)
+{
+BCM2835PowerMgtState *s = (BCM2835PowerMgtState *)opaque;
+
+if ((value & PASSWORD_MASK) != PASSWORD) {
+qemu_log_mask(LOG_GUEST_ERROR,
+  "bcm2835_powermgt_write: Bad password 0x%"PRIx64
+  " at offset 0x%08"HWADDR_PRIx"\n",
+  value, offset);
+return;
+}
+
+value = value & ~PASSWORD_MASK;
+
+switch (offset) {
+case R_RSTC:
+s->rstc = value;
+if (value & V_RSTC_RESET) {
+if ((s->rsts & 0xfff) == V_RSTS_POWEROFF) {
+qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
+} else {
+qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
+}
+}
+break;
+case R_RSTS:
+qemu_log_mask(LOG_UNIMP,
+  "bcm2835_pow

Re: [PATCH qemu] Add basic power management to raspi.

2021-06-02 Thread Nolan

On 6/2/21 4:22 PM, Philippe Mathieu-Daudé wrote:

On 6/2/21 11:33 PM, Nolan wrote:

On 5/31/21 4:23 AM, Philippe Mathieu-Daudé wrote:

Hi Nolan,

Thanks for your patch!

There is something odd with your email address, which apparently
became source...@sigbus.net instead of no...@sigbus.net.


Ugh, oops. I was trying out sourcehut for this, and reflexively gave
them a marker email. I'm pretty sure sourcehut won't sell my email
address, so I'll just change it.


On 5/18/21 10:24 PM, ~nolanl wrote:

From: Nolan Leake 

This is just enough to make reboot and poweroff work.


Please precise "for Linux kernels", since this doesn't work
with firmwares (ideally we should implement the FIRMWARE_NOTIFY_REBOOT
property - which Linux sends - to handle the machine reboot/halt
via the videocore firmware model).


Thanks, good point re: this being tuned to what Linux (and u-boot) do.
Poking around a bit, it looks like
"trusted-firmware-a"/"arm-trusted-firmware" uses the same method, as do
a couple of bare-metal/hobby OSes. Couldn't immediately figure out what
FreeBSD does.

I'm not sure I understand your point about FIRMWARE_NOTIFY_REBOOT, my
understanding is that message is there to tell the GPU firmware that
we're about to reboot, so it can do things like reload the PCIe USB
chip's firmware. In my testing without the watchdog module loaded, my
physical pi4 does not reboot, so it appears that sending
FIRMWARE_NOTIFY_REBOOT is not enough on its own.


 From the ARM core point of view, once it sent the FIRMWARE_NOTIFY_REBOOT
message, it can't really power-off itself, it waits in a busy loop for
the VC to disable its power domain.

hw/misc/bcm2835_property.c is our model of the VC behavior. IMO this
should be where QEMU shuts down. How I'd model it is:

- ARM: sends FIRMWARE_NOTIFY_REBOOT and loops

- VC emulated via property: delays (200ms?) then calls
   SHUTDOWN_CAUSE_GUEST_RESET.

(it helps to see hw/misc/bcm2835_property.c as an external component).


This is not what I see on my hardware pi4. With the following kernel config:
...
CONFIG_RASPBERRYPI_FIRMWARE=y
...
CONFIG_BCM2835_WDT=m
...

if I reboot the machine without the WDT module (but with the firmware 
module), I get this:

#  echo b > /proc/sysrq-trigger
[   54.498768] sysrq: Resetting
[   54.501713] SMP: stopping secondary CPUs
[   54.505701] Reboot failed -- System halted

and it hangs forever there.

If I load the WDT module, it reboots successfully.


qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
+    } else {
+    qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
+    }
+    }


Shouldn't we log the unsupported bits?


I can add that, I didn't originally because the unsupported writes are
expected.


I'd prefer we log them, even if unsupported, so in case something
behaves oddly we could look at those bits.


In v2, I log the writes, since any side effects they have (notably the 
watchdog register) are unimplemented. I didn't log the reads, since they 
actually behave exactly as the real hardware does...



+static void bcm2835_powermgt_reset(DeviceState *dev)
+{
+    BCM2835PowerMgtState *s = BCM2835_POWERMGT(dev);
+
+    s->rstc = 0x0102;
+    s->rsts = 0x1000;
+    s->wdog = 0x;


Where these bits come from?


 From my pi4. https://elinux.org/BCM2835_registers agrees (processed from
Broadcom source code).


OK, so please add a comment referring to
https://elinux.org/BCM2835_registers#PM.

Looking forward for v2 :)


Already sent. Is the link comment here worth a v3?




Re: [PATCH qemu] Add basic power management to raspi.

2021-06-02 Thread Nolan

On 5/31/21 4:23 AM, Philippe Mathieu-Daudé wrote:

Hi Nolan,

Thanks for your patch!

There is something odd with your email address, which apparently
became source...@sigbus.net instead of no...@sigbus.net.


Ugh, oops. I was trying out sourcehut for this, and reflexively gave 
them a marker email. I'm pretty sure sourcehut won't sell my email 
address, so I'll just change it.



On 5/18/21 10:24 PM, ~nolanl wrote:

From: Nolan Leake 

This is just enough to make reboot and poweroff work.


Please precise "for Linux kernels", since this doesn't work
with firmwares (ideally we should implement the FIRMWARE_NOTIFY_REBOOT
property - which Linux sends - to handle the machine reboot/halt
via the videocore firmware model).


Thanks, good point re: this being tuned to what Linux (and u-boot) do. 
Poking around a bit, it looks like 
"trusted-firmware-a"/"arm-trusted-firmware" uses the same method, as do 
a couple of bare-metal/hobby OSes. Couldn't immediately figure out what 
FreeBSD does.


I'm not sure I understand your point about FIRMWARE_NOTIFY_REBOOT, my 
understanding is that message is there to tell the GPU firmware that 
we're about to reboot, so it can do things like reload the PCIe USB 
chip's firmware. In my testing without the watchdog module loaded, my 
physical pi4 does not reboot, so it appears that sending 
FIRMWARE_NOTIFY_REBOOT is not enough on its own.



Notably, the
watchdog timer functionality is not yet implemented.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/64
Signed-off-by: Nolan Leake 
---
  hw/arm/bcm2835_peripherals.c |  13 ++-
  hw/misc/bcm2835_powermgt.c   | 157 +++
  hw/misc/meson.build  |   1 +
  include/hw/arm/bcm2835_peripherals.h |   3 +-
  include/hw/misc/bcm2835_powermgt.h   |  29 +
  5 files changed, 201 insertions(+), 2 deletions(-)
  create mode 100644 hw/misc/bcm2835_powermgt.c
  create mode 100644 include/hw/misc/bcm2835_powermgt.h

diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index dcff13433e..48538c9360 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -126,6 +126,10 @@ static void bcm2835_peripherals_init(Object *obj)
  
  object_property_add_const_link(OBJECT(>dwc2), "dma-mr",

 OBJECT(>gpu_bus_mr));
+
+/* Power Management */
+object_initialize_child(obj, "powermgt", >powermgt,
+TYPE_BCM2835_POWERMGT);
  }
  
  static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)

@@ -364,9 +368,16 @@ static void bcm2835_peripherals_realize(DeviceState *dev, 
Error **errp)
  qdev_get_gpio_in_named(DEVICE(>ic), BCM2835_IC_GPU_IRQ,
 INTERRUPT_USB));
  
+/* Power Management */

+if (!sysbus_realize(SYS_BUS_DEVICE(>powermgt), errp)) {
+return;
+}
+
+memory_region_add_subregion(>peri_mr, PM_OFFSET,
+sysbus_mmio_get_region(SYS_BUS_DEVICE(>powermgt), 0));
+
  create_unimp(s, >txp, "bcm2835-txp", TXP_OFFSET, 0x1000);
  create_unimp(s, >armtmr, "bcm2835-sp804", ARMCTRL_TIMER0_1_OFFSET, 
0x40);
-create_unimp(s, >powermgt, "bcm2835-powermgt", PM_OFFSET, 0x114);
  create_unimp(s, >i2s, "bcm2835-i2s", I2S_OFFSET, 0x100);
  create_unimp(s, >smi, "bcm2835-smi", SMI_OFFSET, 0x100);
  create_unimp(s, >spi[0], "bcm2835-spi0", SPI0_OFFSET, 0x20);
diff --git a/hw/misc/bcm2835_powermgt.c b/hw/misc/bcm2835_powermgt.c
new file mode 100644
index 00..81107ecc8f
--- /dev/null
+++ b/hw/misc/bcm2835_powermgt.c
@@ -0,0 +1,157 @@
+/*
+ * BCM2835 Power Management emulation
+ *
+ * Copyright (C) 2017 Marcin Chojnacki 
+ * Copyright (C) 2021 Nolan Leake 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "hw/misc/bcm2835_powermgt.h"
+#include "migration/vmstate.h"
+#include "sysemu/runstate.h"
+
+#define PASSWORD 0x5a00
+#define PASSWORD_MASK 0xff00
+
+#define R_RSTC 0x1c
+#define V_RSTC_RESET 0x20
+#define R_RSTS 0x20
+#define V_RSTS_POWEROFF 0x555
+#define R_WDOG 0x24
+
+static uint64_t bcm2835_powermgt_read(void *opaque, hwaddr offset,
+  unsigned size)
+{
+BCM2835PowerMgtState *s = (BCM2835PowerMgtState *)opaque;
+uint32_t res = 0;
+
+assert(size == 4);


Instead of this assert, add in bcm2835_powermgt_ops:

   .impl.min_access_size = 4,
   .impl.max_access_size = 4,


Will do.


+
+switch (offset) {
+case R_RSTC:
+res = s->rstc;
+break;
+case R_RSTS:
+res = s->rsts;
+break;
+case R_WDO

[Bug 1913926] [NEW] [QEMU User-Mode] Mesa Fails To Load RadeonSI Driver When In Docker Image

2021-01-30 Thread Connor Nolan
Public bug reported:

# System Details
AMD Ryzen 7 3700U
Ubuntu 20.04 Focal Focus

# Dockerfile

FROM arm32v7/debian:bullseye

RUN apt-get update && apt-get install -y mesa-utils

ENTRYPOINT glxgears

# Instructions For Reproduction
1. Install Docker
2. Build Docker Image: docker build --tag mesa-arm-test .
3. Run: docker run -v /tmp/.X11-unix:/tmp/.X11-unix --device /dev/dri:/dev/dri 
-e "DISPLAY=${DISPLAY}" mesa-arm-test

The Output Is:

amdgpu_device_initialize: amdgpu_query_info(ACCEL_WORKING) failed (-38)
amdgpu: amdgpu_device_initialize failed.
libGL error: failed to create dri screen
libGL error: failed to load driver: radeonsi
libGL error: failed to get magic
libGL error: failed to load driver: radeonsi

It then appears to run using software rendering.

** Affects: qemu
 Importance: Undecided
 Status: New

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1913926

Title:
  [QEMU User-Mode] Mesa Fails To Load RadeonSI Driver When In Docker
  Image

Status in QEMU:
  New

Bug description:
  # System Details
  AMD Ryzen 7 3700U
  Ubuntu 20.04 Focal Focus

  # Dockerfile

  FROM arm32v7/debian:bullseye

  RUN apt-get update && apt-get install -y mesa-utils

  ENTRYPOINT glxgears

  # Instructions For Reproduction
  1. Install Docker
  2. Build Docker Image: docker build --tag mesa-arm-test .
  3. Run: docker run -v /tmp/.X11-unix:/tmp/.X11-unix --device 
/dev/dri:/dev/dri -e "DISPLAY=${DISPLAY}" mesa-arm-test

  The Output Is:

  amdgpu_device_initialize: amdgpu_query_info(ACCEL_WORKING) failed (-38)
  amdgpu: amdgpu_device_initialize failed.
  libGL error: failed to create dri screen
  libGL error: failed to load driver: radeonsi
  libGL error: failed to get magic
  libGL error: failed to load driver: radeonsi

  It then appears to run using software rendering.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1913926/+subscriptions