Re: [PATCH 1/1] hw/arm/sbsa-ref: use XHCI to replace EHCI
On Wed, 31 May 2023, at 5:36 PM, Leif Lindholm wrote: > On 2023-05-31 16:27, Peter Maydell wrote: >> On Wed, 31 May 2023 at 15:58, Graeme Gregory wrote: >>>> The current sbsa-ref cannot use EHCI controller which is only >>>> able to do 32-bit DMA, since sbsa-ref doesn't have RAM above 4GB. >>>> Hence, this uses XHCI to provide a usb controller with 64-bit >>>> DMA capablity instead of EHCI. >>> >>> Should this be below 4G? >> >> It would be pretty disruptive to try to rearrange the memory >> map to put RAM below 4GB at this point, though in theory it's >> possible I guess. (I have a vague recollection that there was >> some reason the RAM was all put above 4GB, but can't find >> anything about that in my email archives. Perhaps Leif remembers?) > > I think Graeme was just pointing out a typo in Marcin's email. > Yes the typo! Graeme > Yeah, we're not changing the DRAM base at this stage. > I think the reason we put no RAM below 4GB was simply that we had > several real-world platforms where that was true, and given the intended > use-case for the platform, we explicitly wanted to trigger issues those > platforms might encounter. > >>> Also has EHCI never worked, or has it worked in some modes and so this >>> change should be versioned? >> >> AIUI, EHCI has never worked and can never have worked, because >> this board's RAM is all above 4G and the QEMU EHCI controller >> implementation only allows DMA descriptors with 32-bit addresses. >> >> Looking back at the archives, it seems we discussed XHCI vs >> EHCI when the sbsa-ref board went in, and the conclusion was >> that XHCI would be better. But there wasn't a sysbus XHCI device >> at that point, so we ended up committing the sbsa-ref board >> with EHCI and a plan to switch to XHCI when the sysbus-xhci >> device was done, which we then forgot about: >> https://mail.gnu.org/archive/html/qemu-arm/2018-11/msg00638.html > > Ah, thanks! That explains why we did the thing that made no sense :) > > To skip the migration hazard, my prefernece is we just leave the EHCI > device in for now, and add a separate XHCI on PCIe. We can drop the > EHCI device at some point in the future. > > / > Leif
Re: [PATCH 1/1] hw/arm/sbsa-ref: use XHCI to replace EHCI
On Wed, May 31, 2023 at 03:02:29PM +0800, wangyuquan1...@phytium.com.cn wrote:
> From: Yuquan Wang
>
> The current sbsa-ref cannot use EHCI controller which is only
> able to do 32-bit DMA, since sbsa-ref doesn't have RAM above 4GB.
> Hence, this uses XHCI to provide a usb controller with 64-bit
> DMA capablity instead of EHCI.
>
Should this be below 4G?
Also has EHCI never worked, or has it worked in some modes and so this
change should be versioned?
Graeme
> Signed-off-by: Yuquan Wang
> Change-Id: I1376f8bbc0e25dcd9d8a22b6e061cb56b3486394
> ---
> hw/arm/sbsa-ref.c | 16
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> index 792371fdce..f9c0647353 100644
> --- a/hw/arm/sbsa-ref.c
> +++ b/hw/arm/sbsa-ref.c
> @@ -81,7 +81,7 @@ enum {
> SBSA_SECURE_UART_MM,
> SBSA_SECURE_MEM,
> SBSA_AHCI,
> -SBSA_EHCI,
> +SBSA_XHCI,
> };
>
> struct SBSAMachineState {
> @@ -118,7 +118,7 @@ static const MemMapEntry sbsa_ref_memmap[] = {
> [SBSA_SMMU] = { 0x6005, 0x0002 },
> /* Space here reserved for more SMMUs */
> [SBSA_AHCI] = { 0x6010, 0x0001 },
> -[SBSA_EHCI] = { 0x6011, 0x0001 },
> +[SBSA_XHCI] = { 0x6011, 0x0001 },
> /* Space here reserved for other devices */
> [SBSA_PCIE_PIO] = { 0x7fff, 0x0001 },
> /* 32-bit address PCIE MMIO space */
> @@ -138,7 +138,7 @@ static const int sbsa_ref_irqmap[] = {
> [SBSA_SECURE_UART] = 8,
> [SBSA_SECURE_UART_MM] = 9,
> [SBSA_AHCI] = 10,
> -[SBSA_EHCI] = 11,
> +[SBSA_XHCI] = 11,
> [SBSA_SMMU] = 12, /* ... to 15 */
> [SBSA_GWDT_WS0] = 16,
> };
> @@ -558,12 +558,12 @@ static void create_ahci(const SBSAMachineState *sms)
> }
> }
>
> -static void create_ehci(const SBSAMachineState *sms)
> +static void create_xhci(const SBSAMachineState *sms)
> {
> -hwaddr base = sbsa_ref_memmap[SBSA_EHCI].base;
> -int irq = sbsa_ref_irqmap[SBSA_EHCI];
> +hwaddr base = sbsa_ref_memmap[SBSA_XHCI].base;
> +int irq = sbsa_ref_irqmap[SBSA_XHCI];
>
> -sysbus_create_simple("platform-ehci-usb", base,
> +sysbus_create_simple("sysbus-xhci", base,
> qdev_get_gpio_in(sms->gic, irq));
> }
>
> @@ -785,7 +785,7 @@ static void sbsa_ref_init(MachineState *machine)
>
> create_ahci(sms);
>
> -create_ehci(sms);
> +create_xhci(sms);
>
> create_pcie(sms);
>
> --
> 2.34.1
>
>
Re: [PATCH 0/9] Add Qualcomm BMC machines
On Thu, Jun 23, 2022 at 08:48:49AM +0200, Cédric Le Goater wrote: > On 6/23/22 07:25, Joel Stanley wrote: > > On Wed, 22 Jun 2022 at 17:29, Jae Hyun Yoo wrote: > > > > > > Hello, > > > > > > I'm sending a series to add Qualcomm BMC machines that are equipped with > > > Aspeed AST2600 SoC. Also, this series adds MAX31785 fan controller device > > > emulation. Please help to review. > > > > Thanks for the MAX31785 model, that's handy to have. > > > > I'm all for more emulation and testing using Qemu models, but I wonder > > if you need to add all three of your boards. They seem to be a > > progression (evb-proto -> dc-scm -> firework). Could you get away with > > just one or two of those? > > I am not sure the evb-proto-bmc is useful to upstream. The other two > are fine. > > Thanks, > I am happy with dropping the evb-proto-bmc machine. We used that internally before actual hardware was available. Graeme > C. > > > > > > > > > > > > > Thanks, > > > > > > Jae > > > > > > Graeme Gregory (2): > > >hw/arm/aspeed: qcom-dc-scm-v1: add block backed FRU device > > >hw/arm/aspeed: add Qualcomm Firework machine and FRU device > > > > > > Jae Hyun Yoo (3): > > >hw/arm/aspeed: add support for the Qualcomm EVB proto board > > >hw/arm/aspeed: add support for the Qualcomm DC-SCM v1 board > > >hw/arm/aspeed: firework: add I2C MUXes for VR channels > > > > > > Maheswara Kurapati (4): > > >hw/i2c: pmbus: Page #255 is valid page for read requests. > > >hw/sensor: add Maxim MAX31785 device > > >hw/arm/aspeed: firework: Add MAX31785 Fan controllers > > >hw/arm/aspeed: firework: Add Thermal Diodes > > > > > > hw/arm/Kconfig| 1 + > > > hw/arm/aspeed.c | 158 +++- > > > hw/i2c/pmbus_device.c | 1 - > > > hw/sensor/Kconfig | 4 + > > > hw/sensor/max31785.c | 580 ++ > > > hw/sensor/meson.build | 1 + > > > 6 files changed, 742 insertions(+), 3 deletions(-) > > > create mode 100644 hw/sensor/max31785.c > > > > > > -- > > > 2.25.1 > > > >
Re: [PATCH v3 0/2] Aspeed I3C device model
On Wed, Jan 12, 2022 at 01:45:05PM +0100, Cédric Le Goater wrote: > Hello Gregory, > > On 1/12/22 11:57, Graeme Gregory wrote: > > On Tue, Jan 11, 2022 at 04:45:44PM +0800, Troy Lee wrote: > > > This series of patch introduce a dummy implemenation of aspeed i3c > > > model, and it provide just enough information for guest machine. > > > However, the driver probing is still failed, but it will not cause > > > kernel panic. > > > > > > > These patches arrived just in time for our i3c testing. This stops > > our CI halting due to kernel panic on i3c probing. > > > > Reviewed-by: Graeme Gregory > > Tested-by: Graeme Gregory > > Excellent ! > > Are you using the Aspeed image from : > > https://github.com/AspeedTech-BMC/openbmc/releases/tag/v07.02 > > or your custom ones ? > We are using the drivers from the v5.4 based SDK currently. Hacked into the v5.15 kernel of openbmc upstream! We needed something quick to test a new PCB. Graeme
Re: [PATCH v3 0/2] Aspeed I3C device model
On Tue, Jan 11, 2022 at 04:45:44PM +0800, Troy Lee wrote: > This series of patch introduce a dummy implemenation of aspeed i3c > model, and it provide just enough information for guest machine. > However, the driver probing is still failed, but it will not cause > kernel panic. > These patches arrived just in time for our i3c testing. This stops our CI halting due to kernel panic on i3c probing. Reviewed-by: Graeme Gregory Tested-by: Graeme Gregory > v3: > - Remove unused AspeedI3CClass > - Refine memory region > - Refine register reset > - Remove unrelated changes to SPI2 address > - Remove i3c controller irq line > > v2: > - Split i3c model into i3c and i3c_device > - Create 6x i3c devices > - Using register fields macro > - Rebase to mainline QEMU > > Troy Lee (2): > Introduce a dummy AST2600 I3C model. > This patch includes i3c instance in ast2600 soc. > > hw/arm/aspeed_ast2600.c | 16 ++ > hw/misc/aspeed_i3c.c | 381 +++ > hw/misc/meson.build | 1 + > hw/misc/trace-events | 6 + > include/hw/arm/aspeed_soc.h | 3 + > include/hw/misc/aspeed_i3c.h | 48 + > 6 files changed, 455 insertions(+) > create mode 100644 hw/misc/aspeed_i3c.c > create mode 100644 include/hw/misc/aspeed_i3c.h > > -- > 2.25.1 > >
Re: [PATCH v5 00/11] hvf: Implement Apple Silicon Support
On Fri, Dec 11, 2020 at 04:12:49PM +0100, Alexander Graf wrote: > Now that Apple Silicon is widely available, people are obviously excited > to try and run virtualized workloads on them, such as Linux and Windows. > > This patch set implements a fully functional version to get the ball > going on that. With this applied, I can successfully run both Linux and > Windows as guests. I am not aware of any limitations specific to > Hypervisor.framework apart from: > > - Live migration / savevm > - gdbstub debugging (SP register) > > > Enjoy! I have been using a VM created using v4/v5 as my daily work machine since v4 came out so. Tested-by: Graeme Gregory > > Alex > > v1 -> v2: > > - New patch: hvf: Actually set SIG_IPI mask > - New patch: hvf: Introduce hvf vcpu struct > - New patch: hvf: arm: Mark CPU as dirty on reset > - Removed patch: hw/arm/virt: Disable highmem when on hypervisor.framework > - Removed patch: arm: Synchronize CPU on PSCI on > - Fix build on 32bit arm > - Merge vcpu kick function patch into ARM enablement > - Implement WFI handling (allows vCPUs to sleep) > - Synchronize system registers (fixes OVMF crashes and reboot) > - Don't always call cpu_synchronize_state() > - Use more fine grained iothread locking > - Populate aa64mmfr0 from hardware > - Make safe to ctrl-C entitlement application > > v2 -> v3: > > - Removed patch: hvf: Actually set SIG_IPI mask > - New patch: hvf: arm: Add support for GICv3 > - New patch: hvf: arm: Implement -cpu host > - Advance PC on SMC > - Use cp list interface for sysreg syncs > - Do not set current_cpu > - Fix sysreg isread mask > - Move sysreg handling to functions > - Remove WFI logic again > - Revert to global iothread locking > > v3 -> v4: > > - Removed patch: hvf: arm: Mark CPU as dirty on reset > - New patch: hvf: Simplify post reset/init/loadvm hooks > - Remove i386-softmmu target (meson.build for hvf target) > - Combine both if statements (PSCI) > - Use hv.h instead of Hypervisor.h for 10.15 compat > - Remove manual inclusion of Hypervisor.h in common .c files > - No longer include Hypervisor.h in arm hvf .c files > - Remove unused exe_full variable > - Reuse exe_name variable > > v4 -> v5: > > - Use g_free() on destroy > > Alexander Graf (10): > hvf: Add hypervisor entitlement to output binaries > hvf: x86: Remove unused definitions > hvf: Move common code out > hvf: Introduce hvf vcpu struct > arm: Set PSCI to 0.2 for HVF > hvf: Simplify post reset/init/loadvm hooks > hvf: Add Apple Silicon support > arm: Add Hypervisor.framework build target > hvf: arm: Add support for GICv3 > hvf: arm: Implement -cpu host > > Peter Collingbourne (1): > arm/hvf: Add a WFI handler > > MAINTAINERS | 14 +- > accel/hvf/entitlements.plist | 8 + > accel/hvf/hvf-all.c | 54 +++ > accel/hvf/hvf-cpus.c | 466 +++ > accel/hvf/meson.build| 7 + > accel/meson.build| 1 + > include/hw/core/cpu.h| 3 +- > include/sysemu/hvf.h | 2 + > include/sysemu/hvf_int.h | 66 +++ > meson.build | 40 +- > scripts/entitlement.sh | 13 + > target/arm/cpu.c | 13 +- > target/arm/cpu.h | 2 + > target/arm/hvf/hvf.c | 856 +++ > target/arm/hvf/meson.build | 3 + > target/arm/kvm_arm.h | 2 - > target/arm/meson.build | 2 + > target/i386/hvf/hvf-cpus.c | 131 -- > target/i386/hvf/hvf-cpus.h | 25 - > target/i386/hvf/hvf-i386.h | 49 +- > target/i386/hvf/hvf.c| 462 +++ > target/i386/hvf/meson.build | 1 - > target/i386/hvf/vmx.h| 24 +- > target/i386/hvf/x86.c| 28 +- > target/i386/hvf/x86_descr.c | 26 +- > target/i386/hvf/x86_emu.c| 62 +-- > target/i386/hvf/x86_mmu.c| 4 +- > target/i386/hvf/x86_task.c | 12 +- > target/i386/hvf/x86hvf.c | 224 - > target/i386/hvf/x86hvf.h | 2 - > 30 files changed, 1786 insertions(+), 816 deletions(-) > create mode 100644 accel/hvf/entitlements.plist > create mode 100644 accel/hvf/hvf-all.c > create mode 100644 accel/hvf/hvf-cpus.c > create mode 100644 accel/hvf/meson.build > create mode 100644 include/sysemu/hvf_int.h > create mode 100755 scripts/entitlement.sh > create mode 100644 target/arm/hvf/hvf.c > create mode 100644 target/arm/hvf/meson.build > delete mode 100644 target/i386/hvf/hvf-cpus.c > delete mode 100644 target/i386/hvf/hvf-cpus.h > > -- > 2.24.3 (Apple Git-128) > >
Re: [RFC PATCH] docs: add some notes on the sbsa-ref machine
On Tue, Nov 03, 2020 at 10:47:10AM +, Alex Bennée wrote: > We should at least document what this machine is about. > Looks good to me. Reviewed-by: Graeme Gregory > Cc: Graeme Gregory > Cc: Leif Lindholm > Cc: Hongbo Zhang > Cc: Shashi Mallela > Signed-off-by: Alex Bennée > --- > docs/system/arm/sbsa.rst | 30 ++ > docs/system/target-arm.rst | 1 + > 2 files changed, 31 insertions(+) > create mode 100644 docs/system/arm/sbsa.rst > > diff --git a/docs/system/arm/sbsa.rst b/docs/system/arm/sbsa.rst > new file mode 100644 > index 00..a47c9360de > --- /dev/null > +++ b/docs/system/arm/sbsa.rst > @@ -0,0 +1,30 @@ > +Arm Server Base System Architecture Reference board (``sbsa-ref``) > +== > + > +While the `virt` board is a generic board platform that doesn't match > +any real hardware the `sbsa-ref` board intends to look like real > +hardware. The `Server Base System Architecture > +<https://developer.arm.com/documentation/den0029/latest>` defines a > +minimum base line of hardware support and importantly how the firmware > +reports that to any operating system. It is a static system that > +reports a very minimal DT to the firmware for command line input to > +the firmware. As a result it must have a firmware specifically built > +to expect a certain hardware layout (as you would in a real machine). > + > +It is intended to be a machine for developing firmware and testing > +standards compliance with operating systems. > + > +Supported devices > +""""""""""""""""" > + > +The sbsa-ref board supports: > + > + - A configurable number of Cortex-A57 cpus > + - GIC version 3 > + - System bus AHCI controller. > + - System bus EHCI controller. > + - CDROM and hard disc on AHCI bus. > + - E1000E ethernet card on PCIE bus. > + - VGA display adaptor on PCIE bus. > + - A generic SBSA watchdog device > + > diff --git a/docs/system/target-arm.rst b/docs/system/target-arm.rst > index fdcf25c237..9636f3fd00 100644 > --- a/docs/system/target-arm.rst > +++ b/docs/system/target-arm.rst > @@ -79,6 +79,7 @@ undocumented; you can get a complete list by running > arm/mps2 > arm/musca > arm/realview > + arm/sbsa-ref > arm/versatile > arm/vexpress > arm/aspeed > -- > 2.20.1 >
Re: [RFC PATCH] hw/arm/virt: use sbsa-ec for reboot and poweroff in secure mode
On Thu, Oct 29, 2020 at 11:19:39AM +, Leif Lindholm wrote: > Hi Peter, (+Ard) > > Graeme is on holiday this week, and I would like his input. > > On Wed, Oct 28, 2020 at 20:31:41 +, Peter Maydell wrote: > > On Wed, 28 Oct 2020 at 08:59, Maxim Uvarov wrote: > > > > > > If we're emulating EL3 then the EL3 guest firmware is responsible for > > > providing the PSCI ABI, including reboot, core power down, etc. > > > sbsa-ref machine has an embedded controller to do reboot, poweroff. > > > Machine > > > virt,secure=on can reuse this code to do reboot inside ATF. > > > > > > Signed-off-by: Maxim Uvarov > > > > (I've cc'd the sbsa-ref machine maintainers.) > > > > > --- > > > Hello, > > > > > > This patch implements reboot for the secure machine inside ATF firmware. > > > I.e. current qemu > > > patch should be used with [1] ATF patch. It looks like that Embedded > > > Controller qemu > > > driver (sbsa-ec) can be common and widely used for other emulated > > > machines. While if > > > there are plans to extend sbsa-ec then we might find some other solution. > > > > > > So for the long term it looks like machine virt was used as an initial > > > playground for secure > > > firmware. While the original intent was a runner for kvm guests. > > > Relation between kvm guest > > > and firmware is not very clear now. If everyone agree it might be good > > > solution to move secure > > > firmware things from virt machine to bsa-ref and make this machine > > > reference for secure boot, > > > firmware updates etc. > > > > > > [1] > > > https://github.com/muvarov/arm-trusted-firmware/commit/6d3339a0081f6f2b45d99bd7e1b67bcbce8f4e0e > > > > > > Thanks for this patch. It is definitely a missing > > bit of functionality that we intend to allow virt to run > > EL3 guest code but don't provide a trigger-shutdown/reboot > > device that works in that setup. > > > > I think the key question here is whether we want to implement > > this by: > > (1) providing the sbsa-ec device in the virt board > > (2) some other mechanism, eg a secure-only pl061 GPIO > > some of whose output pins can trigger shutdown or reboot > > > > The sbsa-ec device has the advantage of doing the > > shutdown/reboot functionality at the moment. The question > > I have for the sbsa-ref board folks is: what are your future > > plans for that device? If the idea is that it's going to end > > up stuffed full of sbsa-ref specific functionality that we > > wouldn't want to try to expose in the virt board, then we > > should probably go with the pl061 approach instead. But if > > it's not likely to grow new functionality then it might be > > simpler... > > > > A couple of notes on this patch if we do go down that route: > > * we would need to arrange to only add the new device for > >new versions of the virt board (that is, the "virt-5.0" > >machine must not have this device because it must look > >like the version of "virt" that shipped with QEMU 5.0) > > * the device needs to be mapped into the Secure address > >space only, because Secure firmware wants control over > >it and doesn't want to allow NS code to reboot the system > >without asking the firmware > > * it would need to be described in the DTB (and maybe also > >ACPI tables? I forget whether we need to describe Secure-only > > devices there) > > Would it? Could it be something that goes into the virt spec? > We don't consume ACPI in firmware (but TF-A will of course have access > to the DT regardless). > > For sbsa-ref, I would ideally like to move to emulating communicating > with an SCP over time, as opposed to TF-A directly controlling the EC. > I am unsure if that leaves much opportunity for code sharing with > virt. > I would agree that would be the ideal end point for the sbsa-ref. I am now kicking myself that the GPIO style solution did not occur to me. I do see the EC device being a stopgap until a proper comms protocol can be implemented to replace it. Graeme
Re: [PATCH v5 2/2] hw/arm/sbsa-ref: add SBSA watchdog device
On Thu, Oct 15, 2020 at 06:21:09PM +0300, Maxim Uvarov wrote:
> On Thu, 15 Oct 2020 at 17:12, Graeme Gregory wrote:
> >
> > On Wed, Oct 14, 2020 at 01:04:43PM -0400, Shashi Mallela wrote:
> > > This was added as a placeholder for the virt requirement suggested by
> > > Maxim
> > > earlier.Agreed that this fdt otherwise has no significance for sbsa-ref
> > > platform nor is being used by ACPI table created for wdt.
> > >
> > > -Shashi
> > >
> > > On Wed, 14 Oct 2020 at 05:31, Graeme Gregory <[1]gra...@nuviainc.com>
> > > wrote:
> > >
> > > On Tue, Oct 13, 2020 at 11:16:31AM -0400, Shashi Mallela wrote:
> > > > Included the newly implemented SBSA generic watchdog device model
> > > into
> > > > SBSA platform
> > > >
> > > > Signed-off-by: Shashi Mallela <[2]shashi.mall...@linaro.org>
> > > > ---
> > > > hw/arm/sbsa-ref.c | 50
> > > +++
> > > > 1 file changed, 50 insertions(+)
> > > >
> > > > diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> > > > index 9c3a893bedfd..97ed41607119 100644
> > > > --- a/hw/arm/sbsa-ref.c
> > > > +++ b/hw/arm/sbsa-ref.c
> > > > @@ -30,6 +30,7 @@
> > > > #include "exec/hwaddr.h"
> > > > #include "kvm_arm.h"
> > > > #include "hw/arm/boot.h"
> > > > +#include "hw/arm/fdt.h"
> > > > #include "hw/block/flash.h"
> > > > #include "hw/boards.h"
> > > > #include "hw/ide/internal.h"
> > > > @@ -40,6 +41,7 @@
> > > > #include "hw/qdev-properties.h"
> > > > #include "hw/usb.h"
> > > > #include "hw/char/pl011.h"
> > > > +#include "hw/watchdog/wdt_sbsa_gwdt.h"
> > > > #include "net/net.h"
> > > > #include "qom/object.h"
> > > >
> > > > @@ -64,6 +66,9 @@ enum {
> > > > SBSA_GIC_DIST,
> > > > SBSA_GIC_REDIST,
> > > > SBSA_SECURE_EC,
> > > > +SBSA_GWDT,
> > > > +SBSA_GWDT_REFRESH,
> > > > +SBSA_GWDT_CONTROL,
> > > > SBSA_SMMU,
> > > > SBSA_UART,
> > > > SBSA_RTC,
> > > > @@ -104,6 +109,8 @@ static const MemMapEntry sbsa_ref_memmap[] = {
> > > > [SBSA_GIC_DIST] = { 0x4006, 0x0001 },
> > > > [SBSA_GIC_REDIST] = { 0x4008, 0x0400 },
> > > > [SBSA_SECURE_EC] = { 0x5000, 0x1000 },
> > > > +[SBSA_GWDT_REFRESH] = { 0x5001, 0x1000 },
> > > > +[SBSA_GWDT_CONTROL] = { 0x50011000, 0x1000 },
> > > > [SBSA_UART] = { 0x6000, 0x1000 },
> > > > [SBSA_RTC] ={ 0x6001, 0x1000 },
> > > > [SBSA_GPIO] = { 0x6002, 0x1000 },
> > > > @@ -133,6 +140,8 @@ static const int sbsa_ref_irqmap[] = {
> > > > [SBSA_SECURE_UART_MM] = 9,
> > > > [SBSA_AHCI] = 10,
> > > > [SBSA_EHCI] = 11,
> > > > +[SBSA_SMMU] = 12, /* ... to 15 */
> > > > +[SBSA_GWDT] = 16,
> > > > };
> >
> > I guess your patch was not based on master here? You should make sure
> > you are rebased to the latest version before sending.
> >
> > > >
> > > > static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms,
> > > int idx)
> > > > @@ -141,6 +150,30 @@ static uint64_t sbsa_ref_cpu_mp_affinity
> > > (SBSAMachineState *sms, int idx)
> > > > return arm_cpu_mp_affinity(idx, clustersz);
> > > > }
> > > >
> > > > +static void create_wdt_fdt(SBSAMachineState *sms)
> > > > +{
> > > > +char *nodename;
> > > > +const char compat[] = "arm,sbsa-gwdt";
> > > > +
> > > > +hwaddr rbase = sbsa_ref_memmap[SBSA_GWDT_REFRESH].base;
> > > > +hwaddr cbase = sbsa_ref_memmap[SBSA_GWDT_CONTROL].base;
> > > > +int irq = sbsa_ref_irqmap[SBSA_GWDT
Re: [PATCH v5 2/2] hw/arm/sbsa-ref: add SBSA watchdog device
On Wed, Oct 14, 2020 at 01:04:43PM -0400, Shashi Mallela wrote:
> This was added as a placeholder for the virt requirement suggested by Maxim
> earlier.Agreed that this fdt otherwise has no significance for sbsa-ref
> platform nor is being used by ACPI table created for wdt.
>
> -Shashi
>
> On Wed, 14 Oct 2020 at 05:31, Graeme Gregory <[1]gra...@nuviainc.com> wrote:
>
> On Tue, Oct 13, 2020 at 11:16:31AM -0400, Shashi Mallela wrote:
> > Included the newly implemented SBSA generic watchdog device model into
> > SBSA platform
> >
> > Signed-off-by: Shashi Mallela <[2]shashi.mall...@linaro.org>
> > ---
> > hw/arm/sbsa-ref.c | 50 +++
> > 1 file changed, 50 insertions(+)
> >
> > diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> > index 9c3a893bedfd..97ed41607119 100644
> > --- a/hw/arm/sbsa-ref.c
> > +++ b/hw/arm/sbsa-ref.c
> > @@ -30,6 +30,7 @@
> > #include "exec/hwaddr.h"
> > #include "kvm_arm.h"
> > #include "hw/arm/boot.h"
> > +#include "hw/arm/fdt.h"
> > #include "hw/block/flash.h"
> > #include "hw/boards.h"
> > #include "hw/ide/internal.h"
> > @@ -40,6 +41,7 @@
> > #include "hw/qdev-properties.h"
> > #include "hw/usb.h"
> > #include "hw/char/pl011.h"
> > +#include "hw/watchdog/wdt_sbsa_gwdt.h"
> > #include "net/net.h"
> > #include "qom/object.h"
> >
> > @@ -64,6 +66,9 @@ enum {
> > SBSA_GIC_DIST,
> > SBSA_GIC_REDIST,
> > SBSA_SECURE_EC,
> > + SBSA_GWDT,
> > + SBSA_GWDT_REFRESH,
> > + SBSA_GWDT_CONTROL,
> > SBSA_SMMU,
> > SBSA_UART,
> > SBSA_RTC,
> > @@ -104,6 +109,8 @@ static const MemMapEntry sbsa_ref_memmap[] = {
> > [SBSA_GIC_DIST] = { 0x4006, 0x0001 },
> > [SBSA_GIC_REDIST] = { 0x4008, 0x0400 },
> > [SBSA_SECURE_EC] = { 0x5000, 0x1000 },
> > + [SBSA_GWDT_REFRESH] = { 0x5001, 0x1000 },
> > + [SBSA_GWDT_CONTROL] = { 0x50011000, 0x1000 },
> > [SBSA_UART] = { 0x6000, 0x1000 },
> > [SBSA_RTC] = { 0x6001, 0x1000 },
> > [SBSA_GPIO] = { 0x6002, 0x1000 },
> > @@ -133,6 +140,8 @@ static const int sbsa_ref_irqmap[] = {
> > [SBSA_SECURE_UART_MM] = 9,
> > [SBSA_AHCI] = 10,
> > [SBSA_EHCI] = 11,
> > + [SBSA_SMMU] = 12, /* ... to 15 */
> > + [SBSA_GWDT] = 16,
> > };
I guess your patch was not based on master here? You should make sure
you are rebased to the latest version before sending.
> >
> > static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int
> idx)
> > @@ -141,6 +150,30 @@ static uint64_t sbsa_ref_cpu_mp_affinity
> (SBSAMachineState *sms, int idx)
> > return arm_cpu_mp_affinity(idx, clustersz);
> > }
> >
> > +static void create_wdt_fdt(SBSAMachineState *sms)
> > +{
> > + char *nodename;
> > + const char compat[] = "arm,sbsa-gwdt";
> > +
> > + hwaddr rbase = sbsa_ref_memmap[SBSA_GWDT_REFRESH].base;
> > + hwaddr cbase = sbsa_ref_memmap[SBSA_GWDT_CONTROL].base;
> > + int irq = sbsa_ref_irqmap[SBSA_GWDT];
> > +
> > + nodename = g_strdup_printf("/watchdog@%" PRIx64, rbase);
> > + qemu_fdt_add_subnode(sms->fdt, nodename);
> > +
> > + qemu_fdt_setprop(sms->fdt, nodename, "compatible",
> > + compat, sizeof(compat));
> > + qemu_fdt_setprop_sized_cells(sms->fdt, nodename, "reg",
> > + 2, rbase, 2, SBSA_GWDT_RMMIO_SIZE,
> > + 2, cbase, 2, SBSA_GWDT_CMMIO_SIZE);
> > + qemu_fdt_setprop_cells(sms->fdt, nodename, "interrupts",
> > + GIC_FDT_IRQ_TYPE_PPI, irq,
> > + GIC_FDT_IRQ_FLAGS_LEVEL_HI);
> > + qemu_fdt_setprop_cell(sms->fdt, nodename, "timeout-sec", 30);
> > + g_free(nodename);
> > +}
> > +
>
> Is this actually us
Re: [PATCH v5 2/2] hw/arm/sbsa-ref: add SBSA watchdog device
On Tue, Oct 13, 2020 at 11:16:31AM -0400, Shashi Mallela wrote:
> Included the newly implemented SBSA generic watchdog device model into
> SBSA platform
>
> Signed-off-by: Shashi Mallela
> ---
> hw/arm/sbsa-ref.c | 50 +++
> 1 file changed, 50 insertions(+)
>
> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> index 9c3a893bedfd..97ed41607119 100644
> --- a/hw/arm/sbsa-ref.c
> +++ b/hw/arm/sbsa-ref.c
> @@ -30,6 +30,7 @@
> #include "exec/hwaddr.h"
> #include "kvm_arm.h"
> #include "hw/arm/boot.h"
> +#include "hw/arm/fdt.h"
> #include "hw/block/flash.h"
> #include "hw/boards.h"
> #include "hw/ide/internal.h"
> @@ -40,6 +41,7 @@
> #include "hw/qdev-properties.h"
> #include "hw/usb.h"
> #include "hw/char/pl011.h"
> +#include "hw/watchdog/wdt_sbsa_gwdt.h"
> #include "net/net.h"
> #include "qom/object.h"
>
> @@ -64,6 +66,9 @@ enum {
> SBSA_GIC_DIST,
> SBSA_GIC_REDIST,
> SBSA_SECURE_EC,
> +SBSA_GWDT,
> +SBSA_GWDT_REFRESH,
> +SBSA_GWDT_CONTROL,
> SBSA_SMMU,
> SBSA_UART,
> SBSA_RTC,
> @@ -104,6 +109,8 @@ static const MemMapEntry sbsa_ref_memmap[] = {
> [SBSA_GIC_DIST] = { 0x4006, 0x0001 },
> [SBSA_GIC_REDIST] = { 0x4008, 0x0400 },
> [SBSA_SECURE_EC] = { 0x5000, 0x1000 },
> +[SBSA_GWDT_REFRESH] = { 0x5001, 0x1000 },
> +[SBSA_GWDT_CONTROL] = { 0x50011000, 0x1000 },
> [SBSA_UART] = { 0x6000, 0x1000 },
> [SBSA_RTC] ={ 0x6001, 0x1000 },
> [SBSA_GPIO] = { 0x6002, 0x1000 },
> @@ -133,6 +140,8 @@ static const int sbsa_ref_irqmap[] = {
> [SBSA_SECURE_UART_MM] = 9,
> [SBSA_AHCI] = 10,
> [SBSA_EHCI] = 11,
> +[SBSA_SMMU] = 12, /* ... to 15 */
> +[SBSA_GWDT] = 16,
> };
>
> static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx)
> @@ -141,6 +150,30 @@ static uint64_t
> sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx)
> return arm_cpu_mp_affinity(idx, clustersz);
> }
>
> +static void create_wdt_fdt(SBSAMachineState *sms)
> +{
> +char *nodename;
> +const char compat[] = "arm,sbsa-gwdt";
> +
> +hwaddr rbase = sbsa_ref_memmap[SBSA_GWDT_REFRESH].base;
> +hwaddr cbase = sbsa_ref_memmap[SBSA_GWDT_CONTROL].base;
> +int irq = sbsa_ref_irqmap[SBSA_GWDT];
> +
> +nodename = g_strdup_printf("/watchdog@%" PRIx64, rbase);
> +qemu_fdt_add_subnode(sms->fdt, nodename);
> +
> +qemu_fdt_setprop(sms->fdt, nodename, "compatible",
> + compat, sizeof(compat));
> +qemu_fdt_setprop_sized_cells(sms->fdt, nodename, "reg",
> + 2, rbase, 2, SBSA_GWDT_RMMIO_SIZE,
> + 2, cbase, 2, SBSA_GWDT_CMMIO_SIZE);
> +qemu_fdt_setprop_cells(sms->fdt, nodename, "interrupts",
> +GIC_FDT_IRQ_TYPE_PPI, irq,
> +GIC_FDT_IRQ_FLAGS_LEVEL_HI);
> +qemu_fdt_setprop_cell(sms->fdt, nodename, "timeout-sec", 30);
> +g_free(nodename);
> +}
> +
Is this actually used anywhere? I ask because SBSA-ref is not a FDT
booting machine and only uses FDT to transfer some dynamic info to
arm-tf/edk2 and is not a full description tree. Your ACPI patch in
edk2 certainly does not use this info.
Graeme
> /*
> * Firmware on this machine only uses ACPI table to load OS, these limited
> * device tree nodes are just to let firmware know the info which varies from
> @@ -219,6 +252,7 @@ static void create_fdt(SBSAMachineState *sms)
>
> g_free(nodename);
> }
> +create_wdt_fdt(sms);
> }
>
> #define SBSA_FLASH_SECTOR_SIZE (256 * KiB)
> @@ -447,6 +481,20 @@ static void create_rtc(const SBSAMachineState *sms)
> sysbus_create_simple("pl031", base, qdev_get_gpio_in(sms->gic, irq));
> }
>
> +static void create_wdt(const SBSAMachineState *sms)
> +{
> +hwaddr rbase = sbsa_ref_memmap[SBSA_GWDT_REFRESH].base;
> +hwaddr cbase = sbsa_ref_memmap[SBSA_GWDT_CONTROL].base;
> +DeviceState *dev = qdev_new(TYPE_WDT_SBSA_GWDT);
> +SysBusDevice *s = SYS_BUS_DEVICE(dev);
> +int irq = sbsa_ref_irqmap[SBSA_GWDT];
> +
> +sysbus_realize_and_unref(s, &error_fatal);
> +sysbus_mmio_map(s, 0, rbase);
> +sysbus_mmio_map(s, 1, cbase);
> +sysbus_connect_irq(s, 0, qdev_get_gpio_in(sms->gic, irq));
> +}
> +
> static DeviceState *gpio_key_dev;
> static void sbsa_ref_powerdown_req(Notifier *n, void *opaque)
> {
> @@ -730,6 +778,8 @@ static void sbsa_ref_init(MachineState *machine)
>
> create_rtc(sms);
>
> +create_wdt(sms);
> +
> create_gpio(sms);
>
> create_ahci(sms);
> --
> 2.18.4
>
>
Re: [PATCH v2 1/2] hw/arm/sbsa-ref : Fix SMMUv3 Initialisation
On Wed, Oct 07, 2020 at 12:44:43PM +0200, Philippe Mathieu-Daudé wrote:
> On 10/7/20 12:32 PM, Graeme Gregory wrote:
> > On Wed, Oct 07, 2020 at 12:24:32PM +0200, Philippe Mathieu-Daudé wrote:
> >> On 10/7/20 12:07 PM, Graeme Gregory wrote:
> >>> SMMUv3 has an error in a previous patch where an i was transposed to a 1
> >>> meaning interrupts would not have been correctly assigned to the SMMUv3
> >>> instance.
> >>>
> >>> Fixes: 48ba18e6d3f3 ("hw/arm/sbsa-ref: Simplify by moving the gic in the
> >>> machine state")
> >>> Signed-off-by: Graeme Gregory
> >>
> >> Again, this fix is already in Peter's queue:
> >> https://www.mail-archive.com/qemu-devel@nongnu.org/msg732819.html
> >>
> >> But if you repost, please collect the reviewer tags,
> >> so we don't have to review it again. This one has:
> >> Reviewed-by: Philippe Mathieu-Daudé
> >> Reviewed-by: Eric Auger
> >>
> >
> > Ah I thought splitting the patch invalidated Eric's reviewed by?
> >
> > This is a different fix to the one you are referring to, previous one
> > was in PCIe.
> >
> > Apologies if I have missed an email from you but I have not received a
> > Reviewed by from you for the SMMUv3 IRQ fix.
>
> Oops sorry my bad, I thought this was a repost of the PCIe fix.
>
> So:
> Reviewed-by: Philippe Mathieu-Daudé
>
> Looking at 48ba18e6d3f3 I messed create_smmu() and
> create_pcie() but you now fixed both cases.
>
> And you are right, this patch isn't reviewed by Eric.
>
Thanks, they are confusingly similar patches.
Graeme
> >
> > Thanks
> >
> > Graeme
> >
> >> Thanks,
> >>
> >> Phil.
> >>
> >>> ---
> >>> hw/arm/sbsa-ref.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> >>> index 9c3a893bed..65e64883b5 100644
> >>> --- a/hw/arm/sbsa-ref.c
> >>> +++ b/hw/arm/sbsa-ref.c
> >>> @@ -525,7 +525,7 @@ static void create_smmu(const SBSAMachineState *sms,
> >>> PCIBus *bus)
> >>> sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
> >>> for (i = 0; i < NUM_SMMU_IRQS; i++) {
> >>> sysbus_connect_irq(SYS_BUS_DEVICE(dev), i,
> >>> - qdev_get_gpio_in(sms->gic, irq + 1));
> >>> + qdev_get_gpio_in(sms->gic, irq + i));
> >>> }
> >>> }
> >>>
> >>>
> >
Re: [PATCH v2 1/2] hw/arm/sbsa-ref : Fix SMMUv3 Initialisation
On Wed, Oct 07, 2020 at 12:24:32PM +0200, Philippe Mathieu-Daudé wrote:
> On 10/7/20 12:07 PM, Graeme Gregory wrote:
> > SMMUv3 has an error in a previous patch where an i was transposed to a 1
> > meaning interrupts would not have been correctly assigned to the SMMUv3
> > instance.
> >
> > Fixes: 48ba18e6d3f3 ("hw/arm/sbsa-ref: Simplify by moving the gic in the
> > machine state")
> > Signed-off-by: Graeme Gregory
>
> Again, this fix is already in Peter's queue:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg732819.html
>
> But if you repost, please collect the reviewer tags,
> so we don't have to review it again. This one has:
> Reviewed-by: Philippe Mathieu-Daudé
> Reviewed-by: Eric Auger
>
Ah I thought splitting the patch invalidated Eric's reviewed by?
This is a different fix to the one you are referring to, previous one
was in PCIe.
Apologies if I have missed an email from you but I have not received a
Reviewed by from you for the SMMUv3 IRQ fix.
Thanks
Graeme
> Thanks,
>
> Phil.
>
> > ---
> > hw/arm/sbsa-ref.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> > index 9c3a893bed..65e64883b5 100644
> > --- a/hw/arm/sbsa-ref.c
> > +++ b/hw/arm/sbsa-ref.c
> > @@ -525,7 +525,7 @@ static void create_smmu(const SBSAMachineState *sms,
> > PCIBus *bus)
> > sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
> > for (i = 0; i < NUM_SMMU_IRQS; i++) {
> > sysbus_connect_irq(SYS_BUS_DEVICE(dev), i,
> > - qdev_get_gpio_in(sms->gic, irq + 1));
> > + qdev_get_gpio_in(sms->gic, irq + i));
> > }
> > }
> >
> >
[PATCH v2 1/2] hw/arm/sbsa-ref : Fix SMMUv3 Initialisation
SMMUv3 has an error in a previous patch where an i was transposed to a 1
meaning interrupts would not have been correctly assigned to the SMMUv3
instance.
Fixes: 48ba18e6d3f3 ("hw/arm/sbsa-ref: Simplify by moving the gic in the
machine state")
Signed-off-by: Graeme Gregory
---
hw/arm/sbsa-ref.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index 9c3a893bed..65e64883b5 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -525,7 +525,7 @@ static void create_smmu(const SBSAMachineState *sms, PCIBus
*bus)
sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
for (i = 0; i < NUM_SMMU_IRQS; i++) {
sysbus_connect_irq(SYS_BUS_DEVICE(dev), i,
- qdev_get_gpio_in(sms->gic, irq + 1));
+ qdev_get_gpio_in(sms->gic, irq + i));
}
}
--
2.25.1
[PATCH v2 0/2] hw/arm/sbsa-ref : small fixes to smmuv3 initialisation
Fix two issues with the smmuv3 initialisation, first where a previous patch had transposed an i to a 1. The second an assumption that the IRQs allocated were meant to be unique and not 0 based. v1->v2 - split into two patches - previously sent as v1 post split by accident, resending for clarity - added Reviewed-by Graeme Gregory (2): hw/arm/sbsa-ref : Fix SMMUv3 Initialisation hw/arm/sbsa-ref : allocate IRQs for SMMUv3 hw/arm/sbsa-ref.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) -- 2.25.1
[PATCH v2 2/2] hw/arm/sbsa-ref : allocate IRQs for SMMUv3
Original commit did not allocate IRQs for the SMMUv3 in the irqmap
effectively using irq 0->3 (shared with other devices). Assuming
original intent was to allocate unique IRQs then add an allocation
to the irqmap.
Fixes: e9fdf453240 ("hw/arm: Add arm SBSA reference machine, devices part")
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Graeme Gregory
---
hw/arm/sbsa-ref.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index 65e64883b5..01863510d0 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -133,6 +133,7 @@ static const int sbsa_ref_irqmap[] = {
[SBSA_SECURE_UART_MM] = 9,
[SBSA_AHCI] = 10,
[SBSA_EHCI] = 11,
+[SBSA_SMMU] = 12, /* ... to 15 */
};
static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx)
--
2.25.1
Re: [RFC PATCH 10/21] contrib/gitdm: Add Nuvia to the domain map
On Sun, Oct 04, 2020 at 08:04:32PM +0200, Philippe Mathieu-Daudé wrote: > There is a number of contributions from this domain, > add its own entry to the gitdm domain map. > > Cc: Graeme Gregory > Cc: Leif Lindholm > Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Graeme Gregory > --- > One Reviewed-by/Ack-by from someone from this domain > should be sufficient to get this patch merged. > --- > contrib/gitdm/domain-map | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/contrib/gitdm/domain-map b/contrib/gitdm/domain-map > index 39251fd97c..d7dca5efd4 100644 > --- a/contrib/gitdm/domain-map > +++ b/contrib/gitdm/domain-map > @@ -25,6 +25,7 @@ codesourcery.com Mentor Graphics > microsoft.com Microsoft > nokia.com Nokia > nutanix.com Nutanix > +nuviainc.comNUVIA > oracle.com Oracle > proxmox.com Proxmox > redhat.com Red Hat > -- > 2.26.2 >
[PATCH 1/2] hw/arm/sbsa-ref : Fix SMMUv3 Initialisation
SMMUv3 has an error in previous patch where a i was transposed to a 1
meaning interrupts would not have been correctly assigned to the SMMUv3
instance.
Fixes: 48ba18e6d3f3 ("hw/arm/sbsa-ref: Simplify by moving the gic in the
machine state")
Signed-off-by: Graeme Gregory
---
hw/arm/sbsa-ref.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index 257ada9425..47e83252c1 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -530,7 +530,7 @@ static void create_smmu(const SBSAMachineState *sms, PCIBus
*bus)
sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
for (i = 0; i < NUM_SMMU_IRQS; i++) {
sysbus_connect_irq(SYS_BUS_DEVICE(dev), i,
- qdev_get_gpio_in(sms->gic, irq + 1));
+ qdev_get_gpio_in(sms->gic, irq + i));
}
}
--
2.25.1
[PATCH 2/2] hw/arm/sbsa-ref : allocate IRQs for SMMUv3
Original commit did not allocate IRQs for the SMMUv3 in the irqmap
effectively using irq 0->3 (shared with other devices). Assuming
original intent was to allocate unique IRQs then add an allocation
to the irqmap.
Fixes: e9fdf453240 ("hw/arm: Add arm SBSA reference machine, devices part")
Signed-off-by: Graeme Gregory
---
hw/arm/sbsa-ref.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index 47e83252c1..9109fb58be 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -138,6 +138,7 @@ static const int sbsa_ref_irqmap[] = {
[SBSA_SECURE_UART_MM] = 9,
[SBSA_AHCI] = 10,
[SBSA_EHCI] = 11,
+[SBSA_SMMU] = 12, /* ... to 15 */
};
static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx)
--
2.25.1
[PATCH 0/2] hw/arm/sbsa-ref : small fixes to smmuv3 initialisation
Fix two issues with the smmuv3 initialisation, first where a previous patch had transposed an i to a 1. The second an assumption that the IRQs allocated were meant to be unique and not 0 based. Graeme Gregory (2): hw/arm/sbsa-ref : Fix SMMUv3 Initialisation hw/arm/sbsa-ref : allocate IRQs for SMMUv3 hw/arm/sbsa-ref.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) -- 2.25.1
Re: [PATCH] hw/arm/sbsa-ref : Fix SMMUv3 Initialisation
On Sat, Sep 26, 2020 at 09:45:24AM +0200, Philippe Mathieu-Daudé wrote:
> Hi Gregory,
>
> On 9/25/20 4:00 PM, Auger Eric wrote:
> > Hi Gregory,
> >
> > On 9/25/20 3:39 PM, Graeme Gregory wrote:
> >> SMMUv3 has an error in previous patch where a i was transposed to a 1
> >> meaning interrupts would not have been correctly assigned to the SMMUv3
> >> instance.
>
> This is a first issue, fixing 48ba18e6d3f3.
>
> >>
> >> The code also contained an error in that the IRQs were never allocated
> >> in the irqmap.
>
> This is another issue, not well explained. IIUC IRQs *are* allocated as
> IRQ #0, right?
>
> This fixes commit e9fdf453240 ("hw/arm: Add arm SBSA reference machine,
> devices part"). Can you split this in another patch please? Eventually
> Cc'ing qemu-sta...@nongnu.org as suggested by Peter.
>
Ok I will split and issue v2 ASAP
Thanks
Graeme
> >>
> >> Fixes: 48ba18e6d3f3 ("hw/arm/sbsa-ref: Simplify by moving the gic in the
> >> machine state")
> >> Signed-off-by: Graeme Gregory
> > Reviewed-by: Eric Auger
> >
> > Thanks
> >
> > Eric
> >
> >> ---
> >> hw/arm/sbsa-ref.c | 3 ++-
> >> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> >> index 257ada9425..9109fb58be 100644
> >> --- a/hw/arm/sbsa-ref.c
> >> +++ b/hw/arm/sbsa-ref.c
> >> @@ -138,6 +138,7 @@ static const int sbsa_ref_irqmap[] = {
> >> [SBSA_SECURE_UART_MM] = 9,
> >> [SBSA_AHCI] = 10,
> >> [SBSA_EHCI] = 11,
> >> +[SBSA_SMMU] = 12, /* ... to 15 */
> >> };
> >>
> >> static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx)
> >> @@ -530,7 +531,7 @@ static void create_smmu(const SBSAMachineState *sms,
> >> PCIBus *bus)
> >> sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
> >> for (i = 0; i < NUM_SMMU_IRQS; i++) {
> >> sysbus_connect_irq(SYS_BUS_DEVICE(dev), i,
> >> - qdev_get_gpio_in(sms->gic, irq + 1));
> >> + qdev_get_gpio_in(sms->gic, irq + i));
>
> BTW this fix is already in Peter's queue:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg732819.html
>
> Thanks,
>
> Phil.
>
> >> }
> >> }
> >>
> >>
> >
>
[PATCH] hw/arm/sbsa-ref : Fix SMMUv3 Initialisation
SMMUv3 has an error in previous patch where a i was transposed to a 1
meaning interrupts would not have been correctly assigned to the SMMUv3
instance.
The code also contained an error in that the IRQs were never allocated
in the irqmap.
Fixes: 48ba18e6d3f3 ("hw/arm/sbsa-ref: Simplify by moving the gic in the
machine state")
Signed-off-by: Graeme Gregory
---
hw/arm/sbsa-ref.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index 257ada9425..9109fb58be 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -138,6 +138,7 @@ static const int sbsa_ref_irqmap[] = {
[SBSA_SECURE_UART_MM] = 9,
[SBSA_AHCI] = 10,
[SBSA_EHCI] = 11,
+[SBSA_SMMU] = 12, /* ... to 15 */
};
static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx)
@@ -530,7 +531,7 @@ static void create_smmu(const SBSAMachineState *sms, PCIBus
*bus)
sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
for (i = 0; i < NUM_SMMU_IRQS; i++) {
sysbus_connect_irq(SYS_BUS_DEVICE(dev), i,
- qdev_get_gpio_in(sms->gic, irq + 1));
+ qdev_get_gpio_in(sms->gic, irq + i));
}
}
--
2.25.1
Re: [EXTERNAL] [PATCH] hw/watchdog: Implement generic watchdog device model as per ARM BSAv0.9 hw/arm/sbsa-ref : Include SBSA watchdog device in sbsa-ref platform
On Thu, Sep 17, 2020 at 05:51:33PM -0400, Shashi Mallela wrote:
> Hi Graeme,
>
> In the generated patch file ,there is subject description (as below)before the
> "Signed-off-by: " line.Not sure why it doesn't show up in the mail.
>
> Subject: [PATCH] hw/watchdog: Implement generic watchdog device model as per
> ARM BSAv0.9 hw/arm/sbsa-ref : Include SBSA watchdog device in sbsa-ref
> platform
>
> I do not have link to updated ACPI tables.
I suspect this shows that it really should be two patches.
[PATCH 1/2] hw/watchdog: Implement SBSA watchdog device
More detailed description including spec references.
SoB:
[PATCH 2/2] hw/arm/sbsa-ref: add SBSA watchdog device
More detailed description.
SoB:
The [Patch X/X] bit obviously being automatically added by
format-patch/send-email.
Thanks
Graeme
>
> Thanks
> Shashi
>
> Sent from [1]Mailspring, the best free email app for work
> On Sep 17 2020, at 5:37 pm, Graeme Gregory wrote:
>
> Something still seems to have gone wrong with subject, and description
> of patch is missing? Missed blank line between 1st line and description?
>
> Do you have the link to updated ACPI tables for testing out of interest?
>
> On Thu, Sep 17, 2020 at 02:12:45PM -0400, Shashi Mallela wrote:
> > Signed-off-by: Shashi Mallela
> > ---
> > hw/arm/Kconfig | 1 +
> > hw/arm/sbsa-ref.c | 23 ++
> > hw/watchdog/Kconfig | 4 +
> > hw/watchdog/meson.build | 1 +
> > hw/watchdog/wdt_sbsa_gwdt.c | 343
> > include/hw/watchdog/wdt_sbsa_gwdt.h | 68 ++
> > 6 files changed, 440 insertions(+)
> > create mode 100644 hw/watchdog/wdt_sbsa_gwdt.c
> > create mode 100644 include/hw/watchdog/wdt_sbsa_gwdt.h
> >
> > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> > index f303c6bead25..6b97e64595d3 100644
> > --- a/hw/arm/Kconfig
> > +++ b/hw/arm/Kconfig
> > @@ -210,6 +210,7 @@ config SBSA_REF
> > select PL031 # RTC
> > select PL061 # GPIO
> > select USB_EHCI_SYSBUS
> > + select WDT_SBSA_GWDT
> >
> > config SABRELITE
> > bool
> > diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> > index ac68b4640d0d..652079a12e37 100644
> > --- a/hw/arm/sbsa-ref.c
> > +++ b/hw/arm/sbsa-ref.c
> > @@ -40,6 +40,7 @@
> > #include "hw/qdev-properties.h"
> > #include "hw/usb.h"
> > #include "hw/char/pl011.h"
> > +#include "hw/watchdog/wdt_sbsa_gwdt.h"
> > #include "net/net.h"
> > #include "qom/object.h"
> >
> > @@ -64,6 +65,9 @@ enum {
> > SBSA_GIC_DIST,
> > SBSA_GIC_REDIST,
> > SBSA_SECURE_EC,
> > + SBSA_GWDT,
> > + SBSA_GWDT_REFRESH,
> > + SBSA_GWDT_CONTROL,
> > SBSA_SMMU,
> > SBSA_UART,
> > SBSA_RTC,
> > @@ -111,6 +115,8 @@ static const MemMapEntry sbsa_ref_memmap[] = {
> > [SBSA_GIC_DIST] = { 0x4006, 0x0001 },
> > [SBSA_GIC_REDIST] = { 0x4008, 0x0400 },
> > [SBSA_SECURE_EC] = { 0x5000, 0x1000 },
> > + [SBSA_GWDT_REFRESH] = { 0x5001, 0x1000 },
> > + [SBSA_GWDT_CONTROL] = { 0x50011000, 0x1000 },
> > [SBSA_UART] = { 0x6000, 0x1000 },
> > [SBSA_RTC] = { 0x6001, 0x1000 },
> > [SBSA_GPIO] = { 0x6002, 0x1000 },
> > @@ -140,6 +146,7 @@ static const int sbsa_ref_irqmap[] = {
> > [SBSA_SECURE_UART_MM] = 9,
> > [SBSA_AHCI] = 10,
> > [SBSA_EHCI] = 11,
> > + [SBSA_GWDT] = 12,
> > };
> >
> > static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx)
> > @@ -454,6 +461,20 @@ static void create_rtc(const SBSAMachineState *sms)
> > sysbus_create_simple("pl031", base, qdev_get_gpio_in(sms->gic, irq));
> > }
> >
> > +static void create_wdt(const SBSAMachineState *sms)
> > +{
> > + hwaddr rbase = sbsa_ref_memmap[SBSA_GWDT_REFRESH].base;
> > + hwaddr cbase = sbsa_ref_memmap[SBSA_GWDT_CONTROL].base;
> > + DeviceState *dev = qdev_new(TYPE_WDT_SBSA_GWDT);
> > + SysBusDevice *s = SYS_BUS_DEVICE(dev);
> > + int irq = sbsa_ref_irqmap[SBSA_GWDT];
> > +
> > + sysbus_realize_and_unref(s, &error_fatal);
> > + sysbus_mmio_map(s, 0, rbase);
> > + sysbus_mmio_map(s, 1, cbase);
> > + sysbus_connect_irq(s, 0, qdev_get_gpio_in(sms->gic, irq));
> > +}
> > +
> &
Re: [EXTERNAL] [PATCH] hw/watchdog: Implement generic watchdog device model as per ARM BSAv0.9 hw/arm/sbsa-ref : Include SBSA watchdog device in sbsa-ref platform
Something still seems to have gone wrong with subject, and description
of patch is missing? Missed blank line between 1st line and description?
Do you have the link to updated ACPI tables for testing out of interest?
On Thu, Sep 17, 2020 at 02:12:45PM -0400, Shashi Mallela wrote:
> Signed-off-by: Shashi Mallela
> ---
> hw/arm/Kconfig | 1 +
> hw/arm/sbsa-ref.c | 23 ++
> hw/watchdog/Kconfig | 4 +
> hw/watchdog/meson.build | 1 +
> hw/watchdog/wdt_sbsa_gwdt.c | 343
> include/hw/watchdog/wdt_sbsa_gwdt.h | 68 ++
> 6 files changed, 440 insertions(+)
> create mode 100644 hw/watchdog/wdt_sbsa_gwdt.c
> create mode 100644 include/hw/watchdog/wdt_sbsa_gwdt.h
>
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index f303c6bead25..6b97e64595d3 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -210,6 +210,7 @@ config SBSA_REF
> select PL031 # RTC
> select PL061 # GPIO
> select USB_EHCI_SYSBUS
> +select WDT_SBSA_GWDT
>
> config SABRELITE
> bool
> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> index ac68b4640d0d..652079a12e37 100644
> --- a/hw/arm/sbsa-ref.c
> +++ b/hw/arm/sbsa-ref.c
> @@ -40,6 +40,7 @@
> #include "hw/qdev-properties.h"
> #include "hw/usb.h"
> #include "hw/char/pl011.h"
> +#include "hw/watchdog/wdt_sbsa_gwdt.h"
> #include "net/net.h"
> #include "qom/object.h"
>
> @@ -64,6 +65,9 @@ enum {
> SBSA_GIC_DIST,
> SBSA_GIC_REDIST,
> SBSA_SECURE_EC,
> +SBSA_GWDT,
> +SBSA_GWDT_REFRESH,
> +SBSA_GWDT_CONTROL,
> SBSA_SMMU,
> SBSA_UART,
> SBSA_RTC,
> @@ -111,6 +115,8 @@ static const MemMapEntry sbsa_ref_memmap[] = {
> [SBSA_GIC_DIST] = { 0x4006, 0x0001 },
> [SBSA_GIC_REDIST] = { 0x4008, 0x0400 },
> [SBSA_SECURE_EC] = { 0x5000, 0x1000 },
> +[SBSA_GWDT_REFRESH] = { 0x5001, 0x1000 },
> +[SBSA_GWDT_CONTROL] = { 0x50011000, 0x1000 },
> [SBSA_UART] = { 0x6000, 0x1000 },
> [SBSA_RTC] ={ 0x6001, 0x1000 },
> [SBSA_GPIO] = { 0x6002, 0x1000 },
> @@ -140,6 +146,7 @@ static const int sbsa_ref_irqmap[] = {
> [SBSA_SECURE_UART_MM] = 9,
> [SBSA_AHCI] = 10,
> [SBSA_EHCI] = 11,
> +[SBSA_GWDT] = 12,
> };
>
> static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx)
> @@ -454,6 +461,20 @@ static void create_rtc(const SBSAMachineState *sms)
> sysbus_create_simple("pl031", base, qdev_get_gpio_in(sms->gic, irq));
> }
>
> +static void create_wdt(const SBSAMachineState *sms)
> +{
> +hwaddr rbase = sbsa_ref_memmap[SBSA_GWDT_REFRESH].base;
> +hwaddr cbase = sbsa_ref_memmap[SBSA_GWDT_CONTROL].base;
> +DeviceState *dev = qdev_new(TYPE_WDT_SBSA_GWDT);
> +SysBusDevice *s = SYS_BUS_DEVICE(dev);
> +int irq = sbsa_ref_irqmap[SBSA_GWDT];
> +
> +sysbus_realize_and_unref(s, &error_fatal);
> +sysbus_mmio_map(s, 0, rbase);
> +sysbus_mmio_map(s, 1, cbase);
> +sysbus_connect_irq(s, 0, qdev_get_gpio_in(sms->gic, irq));
> +}
> +
> static DeviceState *gpio_key_dev;
> static void sbsa_ref_powerdown_req(Notifier *n, void *opaque)
> {
> @@ -737,6 +758,8 @@ static void sbsa_ref_init(MachineState *machine)
>
> create_rtc(sms);
>
> +create_wdt(sms);
> +
> create_gpio(sms);
>
> create_ahci(sms);
> diff --git a/hw/watchdog/Kconfig b/hw/watchdog/Kconfig
> index 293209b291d6..94ac0d5b76b1 100644
> --- a/hw/watchdog/Kconfig
> +++ b/hw/watchdog/Kconfig
> @@ -17,3 +17,7 @@ config WDT_DIAG288
>
> config WDT_IMX2
> bool
> +
> +config WDT_SBSA_GWDT
> +bool
> +default y if SBSA_REF
> \ No newline at end of file
> diff --git a/hw/watchdog/meson.build b/hw/watchdog/meson.build
> index 9b8725e64288..a9a23307acfe 100644
> --- a/hw/watchdog/meson.build
> +++ b/hw/watchdog/meson.build
> @@ -5,3 +5,4 @@ softmmu_ss.add(when: 'CONFIG_WDT_IB700', if_true:
> files('wdt_ib700.c'))
> softmmu_ss.add(when: 'CONFIG_WDT_DIAG288', if_true: files('wdt_diag288.c'))
> softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('wdt_aspeed.c'))
> softmmu_ss.add(when: 'CONFIG_WDT_IMX2', if_true: files('wdt_imx2.c'))
> +softmmu_ss.add(when: 'CONFIG_WDT_SBSA_GWDT', if_true:
> files('wdt_sbsa_gwdt.c'))
> diff --git a/hw/watchdog/wdt_sbsa_gwdt.c b/hw/watchdog/wdt_sbsa_gwdt.c
> new file mode 100644
> index ..eb3be0862cf1
> --- /dev/null
> +++ b/hw/watchdog/wdt_sbsa_gwdt.c
> @@ -0,0 +1,343 @@
> +/*
> + * Generic watchdog device model for SBSA
> + *
> + * Copyright Linaro.org 2020
> + *
> + * Authors:
> + * Shashi Mallela
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or (at
> your
> + * option) any later version. See the COPYING file in the top-level
> directory.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "sysemu/reset.h"
> +#include "sysemu/watchdog
Re: [PATCH] hw/arm/sbsa-ref: add "reg" property to DT cpu nodes
On Tue, Aug 25, 2020 at 05:52:17PM +0100, Leif Lindholm wrote:
> The sbsa-ref platform uses a minimal device tree to pass amount of memory
> as well as number of cpus to the firmware. However, when dumping that
> minimal dtb (with -M sbsa-virt,dumpdtb=), the resulting blob
> generates a warning when decompiled by dtc due to lack of reg property.
>
> Add a simple reg property per cpu, representing a 64-bit MPIDR_EL1.
>
> Signed-off-by: Leif Lindholm
> ---
> hw/arm/sbsa-ref.c | 16
> 1 file changed, 16 insertions(+)
>
> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> index f030a416fd..167c57a08b 100644
> --- a/hw/arm/sbsa-ref.c
> +++ b/hw/arm/sbsa-ref.c
> @@ -183,7 +183,22 @@ static void create_fdt(SBSAMachineState *sms)
> g_free(matrix);
> }
>
> +/*
> + * From Documentation/devicetree/bindings/arm/cpus.yaml
> + * On ARM v8 64-bit systems this property is required
> + *and matches the MPIDR_EL1 register affinity bits.
> + *
> + ** If cpus node's #address-cells property is set to 2
> + *
> + * The first reg cell bits [7:0] must be set to
> + * bits [39:32] of MPIDR_EL1.
> + *
> + * The second reg cell bits [23:0] must be set to
> + * bits [23:0] of MPIDR_EL1.
> + */
> qemu_fdt_add_subnode(sms->fdt, "/cpus");
> +qemu_fdt_setprop_cell(sms->fdt, "/cpus", "#address-cells", 2);
> +qemu_fdt_setprop_cell(sms->fdt, "/cpus", "#size-cells", 0x0);
>
> for (cpu = sms->smp_cpus - 1; cpu >= 0; cpu--) {
> char *nodename = g_strdup_printf("/cpus/cpu@%d", cpu);
> @@ -191,6 +206,7 @@ static void create_fdt(SBSAMachineState *sms)
> CPUState *cs = CPU(armcpu);
>
> qemu_fdt_add_subnode(sms->fdt, nodename);
> +qemu_fdt_setprop_u64(sms->fdt, nodename, "reg", cpu);
To obey the statement earlier shouldnt this be taken from the mp_affinity
variable in armcpu?
Graeme
>
> if (ms->possible_cpus->cpus[cs->cpu_index].props.has_node_id) {
> qemu_fdt_setprop_cell(sms->fdt, nodename, "numa-node-id",
> --
> 2.20.1
>
[PATCH v2 1/2] hw/misc/sbsa_ec : Add an embedded controller for sbsa-ref
A difference between sbsa platform and the virt platform is PSCI is
handled by ARM-TF in the sbsa platform. This means that the PSCI code
there needs to communicate some of the platform power changes down
to the qemu code for things like shutdown/reset control.
Space has been left to extend the EC if we find other use cases in
future where ARM-TF and qemu need to communicate.
Signed-off-by: Graeme Gregory
---
hw/misc/meson.build | 2 +
hw/misc/sbsa_ec.c | 98 +
2 files changed, 100 insertions(+)
create mode 100644 hw/misc/sbsa_ec.c
diff --git a/hw/misc/meson.build b/hw/misc/meson.build
index 84fed0494d..e1576b81cf 100644
--- a/hw/misc/meson.build
+++ b/hw/misc/meson.build
@@ -97,3 +97,5 @@ specific_ss.add(when: 'CONFIG_MAC_VIA', if_true:
files('mac_via.c'))
specific_ss.add(when: 'CONFIG_MIPS_CPS', if_true: files('mips_cmgcr.c',
'mips_cpc.c'))
specific_ss.add(when: 'CONFIG_MIPS_ITU', if_true: files('mips_itu.c'))
+
+specific_ss.add(when: 'CONFIG_SBSA_REF', if_true: files('sbsa_ec.c'))
diff --git a/hw/misc/sbsa_ec.c b/hw/misc/sbsa_ec.c
new file mode 100644
index 00..9a7d7f914a
--- /dev/null
+++ b/hw/misc/sbsa_ec.c
@@ -0,0 +1,98 @@
+/*
+ * ARM SBSA Reference Platform Embedded Controller
+ *
+ * A device to allow PSCI running in the secure side of sbsa-ref machine
+ * to communicate platform power states to qemu.
+ *
+ * Copyright (c) 2020 Nuvia Inc
+ * Written by Graeme Gregory
+ *
+ * SPDX-License-Identifer: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "qemu/log.h"
+#include "hw/sysbus.h"
+#include "sysemu/runstate.h"
+
+typedef struct {
+SysBusDevice parent_obj;
+MemoryRegion iomem;
+} SECUREECState;
+
+#define TYPE_SBSA_EC "sbsa-ec"
+#define SECURE_EC(obj) OBJECT_CHECK(SECUREECState, (obj), TYPE_SBSA_EC)
+
+enum sbsa_ec_powerstates {
+SBSA_EC_CMD_POWEROFF = 0x01,
+SBSA_EC_CMD_REBOOT = 0x02,
+};
+
+static uint64_t sbsa_ec_read(void *opaque, hwaddr offset, unsigned size)
+{
+/* No use for this currently */
+qemu_log_mask(LOG_GUEST_ERROR, "sbsa-ec: no readable registers");
+return 0;
+}
+
+static void sbsa_ec_write(void *opaque, hwaddr offset,
+ uint64_t value, unsigned size)
+{
+if (offset == 0) { /* PSCI machine power command register */
+switch (value) {
+case SBSA_EC_CMD_POWEROFF:
+qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
+break;
+case SBSA_EC_CMD_REBOOT:
+qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
+break;
+default:
+qemu_log_mask(LOG_GUEST_ERROR,
+ "sbsa-ec: unknown power command");
+}
+} else {
+qemu_log_mask(LOG_GUEST_ERROR, "sbsa-ec: unknown EC register");
+}
+}
+
+static const MemoryRegionOps sbsa_ec_ops = {
+.read = sbsa_ec_read,
+.write = sbsa_ec_write,
+.endianness = DEVICE_NATIVE_ENDIAN,
+.valid.min_access_size = 4,
+.valid.max_access_size = 4,
+};
+
+static void sbsa_ec_init(Object *obj)
+{
+SECUREECState *s = SECURE_EC(obj);
+SysBusDevice *dev = SYS_BUS_DEVICE(obj);
+
+memory_region_init_io(&s->iomem, obj, &sbsa_ec_ops, s, "sbsa-ec",
+ 0x1000);
+sysbus_init_mmio(dev, &s->iomem);
+}
+
+static void sbsa_ec_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+
+/* No vmstate or reset required: device has no internal state */
+dc->user_creatable = false;
+}
+
+static const TypeInfo sbsa_ec_info = {
+.name = TYPE_SBSA_EC,
+.parent= TYPE_SYS_BUS_DEVICE,
+.instance_size = sizeof(SECUREECState),
+.instance_init = sbsa_ec_init,
+.class_init= sbsa_ec_class_init,
+};
+
+static void sbsa_ec_register_type(void)
+{
+type_register_static(&sbsa_ec_info);
+}
+
+type_init(sbsa_ec_register_type);
--
2.25.1
[PATCH v2 2/2] hw/arm/sbsa-ref : Add embedded controller in secure memory
Add the previously created sbsa-ec device to the sbsa-ref machine in
secure memory so the PSCI implementation in ARM-TF can access it, but
not expose it to non secure firmware or OS except by via ARM-TF.
Signed-off-by: Graeme Gregory
---
hw/arm/sbsa-ref.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index 2a7d9a61fc..8614709bfb 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -62,6 +62,7 @@ enum {
SBSA_CPUPERIPHS,
SBSA_GIC_DIST,
SBSA_GIC_REDIST,
+SBSA_SECURE_EC,
SBSA_SMMU,
SBSA_UART,
SBSA_RTC,
@@ -107,6 +108,7 @@ static const MemMapEntry sbsa_ref_memmap[] = {
[SBSA_CPUPERIPHS] = { 0x4000, 0x0004 },
[SBSA_GIC_DIST] = { 0x4006, 0x0001 },
[SBSA_GIC_REDIST] = { 0x4008, 0x0400 },
+[SBSA_SECURE_EC] = { 0x5000, 0x1000 },
[SBSA_UART] = { 0x6000, 0x1000 },
[SBSA_RTC] ={ 0x6001, 0x1000 },
[SBSA_GPIO] = { 0x6002, 0x1000 },
@@ -585,6 +587,16 @@ static void *sbsa_ref_dtb(const struct arm_boot_info
*binfo, int *fdt_size)
return board->fdt;
}
+static void create_secure_ec(MemoryRegion *mem)
+{
+hwaddr base = sbsa_ref_memmap[SBSA_SECURE_EC].base;
+DeviceState *dev = qdev_new("sbsa-ec");
+SysBusDevice *s = SYS_BUS_DEVICE(dev);
+
+memory_region_add_subregion(mem, base,
+sysbus_mmio_get_region(s, 0));
+}
+
static void sbsa_ref_init(MachineState *machine)
{
unsigned int smp_cpus = machine->smp.cpus;
@@ -708,6 +720,8 @@ static void sbsa_ref_init(MachineState *machine)
create_pcie(sms);
+create_secure_ec(secure_sysmem);
+
sms->bootinfo.ram_size = machine->ram_size;
sms->bootinfo.nb_cpus = smp_cpus;
sms->bootinfo.board_id = -1;
--
2.25.1
[PATCH v2 0/2] Add an embedded controller to sbsa-ref machine
This series is to an add embedded controller to the sbsa-ref machine so that PSCI can communicate platform power states to the platform which in this case is QEMU. v1->v2 - broke out the EC itself as hw/misc/sbsa_ec.c as seperate patch - applied review comments to date
Re: [EXTERNAL] Re: [PATCH] hw/arm/sbsa-ref.c : Add a fake embedded controller
On Fri, Aug 21, 2020 at 03:49:11PM +0200, Philippe Mathieu-Daudé wrote:
> On 8/20/20 3:32 PM, Graeme Gregory wrote:
> > A difference between sbsa platform and the virt platform is PSCI is
> > handled by ARM-TF in the sbsa platform. This means that the PSCI code
> > there needs to communicate some of the platform power changes down
> > to the qemu code for things like shutdown/reset control.
>
> No need to explicit 'fake' in patch subject, almost all emulated
> devices are fake.
>
Noted, will change.
> >
> > Space has been left to extend the EC if we find other use cases in
> > future where ARM-TF and qemu need to communicate.
> >
> > Signed-off-by: Graeme Gregory
> > ---
> > hw/arm/sbsa-ref.c | 95 +++
> > 1 file changed, 95 insertions(+)
> >
> > diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> > index f030a416fd..c8743fc1d0 100644
> > --- a/hw/arm/sbsa-ref.c
> > +++ b/hw/arm/sbsa-ref.c
> > @@ -41,6 +41,7 @@
> > #include "hw/usb.h"
> > #include "hw/char/pl011.h"
> > #include "net/net.h"
> > +#include "migration/vmstate.h"
> >
> > #define RAMLIMIT_GB 8192
> > #define RAMLIMIT_BYTES (RAMLIMIT_GB * GiB)
> > @@ -62,6 +63,7 @@ enum {
> > SBSA_CPUPERIPHS,
> > SBSA_GIC_DIST,
> > SBSA_GIC_REDIST,
> > +SBSA_SECURE_EC,
> > SBSA_SMMU,
> > SBSA_UART,
> > SBSA_RTC,
> > @@ -98,6 +100,14 @@ typedef struct {
> > #define SBSA_MACHINE(obj) \
> > OBJECT_CHECK(SBSAMachineState, (obj), TYPE_SBSA_MACHINE)
> >
> > +typedef struct {
> > +SysBusDevice parent_obj;
> > +MemoryRegion iomem;
> > +} SECUREECState;
> > +
> > +#define TYPE_SECURE_EC "sbsa-secure-ec"
> > +#define SECURE_EC(obj) OBJECT_CHECK(SECUREECState, (obj), TYPE_SECURE_EC)
> > +
> > static const MemMapEntry sbsa_ref_memmap[] = {
> > /* 512M boot ROM */
> > [SBSA_FLASH] = { 0, 0x2000 },
> > @@ -107,6 +117,7 @@ static const MemMapEntry sbsa_ref_memmap[] = {
> > [SBSA_CPUPERIPHS] = { 0x4000, 0x0004 },
> > [SBSA_GIC_DIST] = { 0x4006, 0x0001 },
> > [SBSA_GIC_REDIST] = { 0x4008, 0x0400 },
> > +[SBSA_SECURE_EC] = { 0x5000, 0x1000 },
> > [SBSA_UART] = { 0x6000, 0x1000 },
> > [SBSA_RTC] ={ 0x6001, 0x1000 },
> > [SBSA_GPIO] = { 0x6002, 0x1000 },
> > @@ -585,6 +596,65 @@ static void *sbsa_ref_dtb(const struct arm_boot_info
> > *binfo, int *fdt_size)
> > return board->fdt;
> > }
> >
> > +enum sbsa_secure_ec_powerstates {
> > +SBSA_SECURE_EC_CMD_NULL,
> > +SBSA_SECURE_EC_CMD_POWEROFF,
> > +SBSA_SECURE_EC_CMD_REBOOT,
> > +};
> > +
> > +static uint64_t secure_ec_read(void *opaque, hwaddr offset, unsigned size)
> > +{
> > +/* No use for this currently */
> > +return 0;
> > +}
> > +
> > +static void secure_ec_write(void *opaque, hwaddr offset,
> > + uint64_t value, unsigned size)
> > +{
> > +if (offset == 0) { /* PSCI machine power command register */
> > +switch (value) {
> > +case SBSA_SECURE_EC_CMD_NULL:
> > +break;
> > +case SBSA_SECURE_EC_CMD_POWEROFF:
> > +qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
> > +break;
> > +case SBSA_SECURE_EC_CMD_REBOOT:
> > +qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
> > +break;
> > +default:
> > +error_report("sbsa-ref: ERROR Unkown power command");
> > +}
> > +} else {
> > +error_report("sbsa-ref: unknown EC register");
> > +}
> > +}
> > +
> > +static const MemoryRegionOps secure_ec_ops = {
> > +.read = secure_ec_read,
> > +.write = secure_ec_write,
> > +.endianness = DEVICE_NATIVE_ENDIAN,
> > +};
> > +
> > +static void secure_ec_init(Object *obj)
> > +{
> > +SECUREECState *s = SECURE_EC(obj);
> > +SysBusDevice *dev = SYS_BUS_DEVICE(obj);
> > +
> > +memory_region_init_io(&s->iomem, obj, &secure_ec_ops, s, "secure-ec",
> > +0x1000);
> > +sysbus_init_mmio(dev, &s->iomem);
> > +}
>
Re: [EXTERNAL] Re: [PATCH] hw/arm/sbsa-ref.c : Add a fake embedded controller
On Mon, Aug 24, 2020 at 03:59:38PM +0100, Peter Maydell wrote:
> On Thu, 20 Aug 2020 at 14:32, Graeme Gregory wrote:
> >
> > A difference between sbsa platform and the virt platform is PSCI is
> > handled by ARM-TF in the sbsa platform. This means that the PSCI code
> > there needs to communicate some of the platform power changes down
> > to the qemu code for things like shutdown/reset control.
> >
> > Space has been left to extend the EC if we find other use cases in
> > future where ARM-TF and qemu need to communicate.
> >
> > Signed-off-by: Graeme Gregory
> > ---
> > hw/arm/sbsa-ref.c | 95 +++
> > 1 file changed, 95 insertions(+)
> >
> > diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> > index f030a416fd..c8743fc1d0 100644
> > --- a/hw/arm/sbsa-ref.c
> > +++ b/hw/arm/sbsa-ref.c
> > @@ -41,6 +41,7 @@
> > #include "hw/usb.h"
> > #include "hw/char/pl011.h"
> > #include "net/net.h"
> > +#include "migration/vmstate.h"
> >
> > #define RAMLIMIT_GB 8192
> > #define RAMLIMIT_BYTES (RAMLIMIT_GB * GiB)
> > @@ -62,6 +63,7 @@ enum {
> > SBSA_CPUPERIPHS,
> > SBSA_GIC_DIST,
> > SBSA_GIC_REDIST,
> > +SBSA_SECURE_EC,
> > SBSA_SMMU,
> > SBSA_UART,
> > SBSA_RTC,
> > @@ -98,6 +100,14 @@ typedef struct {
> > #define SBSA_MACHINE(obj) \
> > OBJECT_CHECK(SBSAMachineState, (obj), TYPE_SBSA_MACHINE)
> >
> > +typedef struct {
> > +SysBusDevice parent_obj;
> > +MemoryRegion iomem;
> > +} SECUREECState;
>
> Could you put the definition of the device in its own .c file,
> please (hw/misc/ seems like the right place). We generally
> prefer to keep the board and individual device models
> separate. (Some older code in the tree still has devices
> lurking in source files, but new stuff generally follows
> the rules.)
>
Yes, certainly!
> > +enum sbsa_secure_ec_powerstates {
> > +SBSA_SECURE_EC_CMD_NULL,
> > +SBSA_SECURE_EC_CMD_POWEROFF,
> > +SBSA_SECURE_EC_CMD_REBOOT,
>
> The last two are clear enough, but what's a null power state for?
>
My personal dislike of sending 0 to hardware as its harder to spot
when using scopes/logic analyzers. I can remove it if you wish and
explicitly number the states.
> > +static uint64_t secure_ec_read(void *opaque, hwaddr offset, unsigned size)
> > +{
> > +/* No use for this currently */
> > +return 0;
> > +}
> > +
> > +static void secure_ec_write(void *opaque, hwaddr offset,
> > + uint64_t value, unsigned size)
> > +{
> > +if (offset == 0) { /* PSCI machine power command register */
> > +switch (value) {
> > +case SBSA_SECURE_EC_CMD_NULL:
> > +break;
> > +case SBSA_SECURE_EC_CMD_POWEROFF:
> > +qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
> > +break;
> > +case SBSA_SECURE_EC_CMD_REBOOT:
> > +qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
> > +break;
> > +default:
> > +error_report("sbsa-ref: ERROR Unkown power command");
>
> "unknown".
>
> We prefer qemu_log_mask(LOG_GUEST_ERROR, ...) for logging this
> kind of "guest code did something wrong" situation.
> (you could also do that for attempting to read this w/o register
> if you liked).
>
Excellent that is much better, Ill make that change.
> > +}
> > +} else {
> > +error_report("sbsa-ref: unknown EC register");
>
> similarly here
>
> > +}
> > +}
> > +
> > +static const MemoryRegionOps secure_ec_ops = {
> > +.read = secure_ec_read,
> > +.write = secure_ec_write,
> > +.endianness = DEVICE_NATIVE_ENDIAN,
>
> Consider explicitly specifying the permitted access size
> by setting .valid.min_access_size and .valid.max_access_size
> (restricting guests to only doing 32-bit writes will make
> life easier when you come to add registers later...)
>
That was my original intent so i will do this.
> > +};
> > +
> > +static void secure_ec_init(Object *obj)
> > +{
> > +SECUREECState *s = SECURE_EC(obj);
> > +SysBusDevice *dev = SYS_BUS_DEVICE(obj);
> > +
> > +memory_region_init_io(&s->iomem, obj, &secure_ec_ops, s, "secure-ec",
> > +0x1000);
>
> Minor indent error here.
>
Will fix, just
[PATCH] hw/arm/sbsa-ref: fix typo breaking PCIe IRQs
Fixing a typo in a previous patch that translated an "i" to a 1
and therefore breaking the allocation of PCIe interrupts. This was
discovered when virtio-net-pci devices ceased to function correctly.
Fixes: 48ba18e6d3f3 ("hw/arm/sbsa-ref: Simplify by moving the gic in the
machine state")
Signed-off-by: Graeme Gregory
---
hw/arm/sbsa-ref.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index 570faac6e2..48c7cea291 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -565,7 +565,7 @@ static void create_pcie(SBSAMachineState *sms)
for (i = 0; i < GPEX_NUM_IRQS; i++) {
sysbus_connect_irq(SYS_BUS_DEVICE(dev), i,
- qdev_get_gpio_in(sms->gic, irq + 1));
+ qdev_get_gpio_in(sms->gic, irq + i));
gpex_set_irq_num(GPEX_HOST(dev), i, irq + i);
}
--
2.25.1
[PATCH] hw/arm/sbsa-ref.c : Add a fake embedded controller
A difference between sbsa platform and the virt platform is PSCI is
handled by ARM-TF in the sbsa platform. This means that the PSCI code
there needs to communicate some of the platform power changes down
to the qemu code for things like shutdown/reset control.
Space has been left to extend the EC if we find other use cases in
future where ARM-TF and qemu need to communicate.
Signed-off-by: Graeme Gregory
---
hw/arm/sbsa-ref.c | 95 +++
1 file changed, 95 insertions(+)
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index f030a416fd..c8743fc1d0 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -41,6 +41,7 @@
#include "hw/usb.h"
#include "hw/char/pl011.h"
#include "net/net.h"
+#include "migration/vmstate.h"
#define RAMLIMIT_GB 8192
#define RAMLIMIT_BYTES (RAMLIMIT_GB * GiB)
@@ -62,6 +63,7 @@ enum {
SBSA_CPUPERIPHS,
SBSA_GIC_DIST,
SBSA_GIC_REDIST,
+SBSA_SECURE_EC,
SBSA_SMMU,
SBSA_UART,
SBSA_RTC,
@@ -98,6 +100,14 @@ typedef struct {
#define SBSA_MACHINE(obj) \
OBJECT_CHECK(SBSAMachineState, (obj), TYPE_SBSA_MACHINE)
+typedef struct {
+SysBusDevice parent_obj;
+MemoryRegion iomem;
+} SECUREECState;
+
+#define TYPE_SECURE_EC "sbsa-secure-ec"
+#define SECURE_EC(obj) OBJECT_CHECK(SECUREECState, (obj), TYPE_SECURE_EC)
+
static const MemMapEntry sbsa_ref_memmap[] = {
/* 512M boot ROM */
[SBSA_FLASH] = { 0, 0x2000 },
@@ -107,6 +117,7 @@ static const MemMapEntry sbsa_ref_memmap[] = {
[SBSA_CPUPERIPHS] = { 0x4000, 0x0004 },
[SBSA_GIC_DIST] = { 0x4006, 0x0001 },
[SBSA_GIC_REDIST] = { 0x4008, 0x0400 },
+[SBSA_SECURE_EC] = { 0x5000, 0x1000 },
[SBSA_UART] = { 0x6000, 0x1000 },
[SBSA_RTC] ={ 0x6001, 0x1000 },
[SBSA_GPIO] = { 0x6002, 0x1000 },
@@ -585,6 +596,65 @@ static void *sbsa_ref_dtb(const struct arm_boot_info
*binfo, int *fdt_size)
return board->fdt;
}
+enum sbsa_secure_ec_powerstates {
+SBSA_SECURE_EC_CMD_NULL,
+SBSA_SECURE_EC_CMD_POWEROFF,
+SBSA_SECURE_EC_CMD_REBOOT,
+};
+
+static uint64_t secure_ec_read(void *opaque, hwaddr offset, unsigned size)
+{
+/* No use for this currently */
+return 0;
+}
+
+static void secure_ec_write(void *opaque, hwaddr offset,
+ uint64_t value, unsigned size)
+{
+if (offset == 0) { /* PSCI machine power command register */
+switch (value) {
+case SBSA_SECURE_EC_CMD_NULL:
+break;
+case SBSA_SECURE_EC_CMD_POWEROFF:
+qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
+break;
+case SBSA_SECURE_EC_CMD_REBOOT:
+qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
+break;
+default:
+error_report("sbsa-ref: ERROR Unkown power command");
+}
+} else {
+error_report("sbsa-ref: unknown EC register");
+}
+}
+
+static const MemoryRegionOps secure_ec_ops = {
+.read = secure_ec_read,
+.write = secure_ec_write,
+.endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void secure_ec_init(Object *obj)
+{
+SECUREECState *s = SECURE_EC(obj);
+SysBusDevice *dev = SYS_BUS_DEVICE(obj);
+
+memory_region_init_io(&s->iomem, obj, &secure_ec_ops, s, "secure-ec",
+0x1000);
+sysbus_init_mmio(dev, &s->iomem);
+}
+
+static void create_secure_ec(MemoryRegion *mem)
+{
+hwaddr base = sbsa_ref_memmap[SBSA_SECURE_EC].base;
+DeviceState *dev = qdev_create(NULL, TYPE_SECURE_EC);
+SysBusDevice *s = SYS_BUS_DEVICE(dev);
+
+memory_region_add_subregion(mem, base,
+sysbus_mmio_get_region(s, 0));
+}
+
static void sbsa_ref_init(MachineState *machine)
{
unsigned int smp_cpus = machine->smp.cpus;
@@ -708,6 +778,8 @@ static void sbsa_ref_init(MachineState *machine)
create_pcie(sms);
+create_secure_ec(secure_sysmem);
+
sms->bootinfo.ram_size = machine->ram_size;
sms->bootinfo.nb_cpus = smp_cpus;
sms->bootinfo.board_id = -1;
@@ -798,8 +870,31 @@ static const TypeInfo sbsa_ref_info = {
.instance_size = sizeof(SBSAMachineState),
};
+static const VMStateDescription vmstate_secure_ec_info = {
+.name = "sbsa-secure-ec",
+.version_id = 0,
+.minimum_version_id = 0,
+};
+
+static void secure_ec_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+
+dc->vmsd = &vmstate_secure_ec_info;
+dc->user_creatable = false;
+}
+
+static const TypeInfo secure_ec_info = {
+.name = TYPE_SECURE_EC,
+.parent= TYPE_SYS_BUS_DEVICE,
+.instance_size = sizeof(SECUREE
Re: [PATCH] hw/arm/sbsa-ref.c : Add a fake embedded controller
On Thu, Aug 20, 2020 at 02:32:01PM +0100, Graeme Gregory wrote:
> A difference between sbsa platform and the virt platform is PSCI is
> handled by ARM-TF in the sbsa platform. This means that the PSCI code
> there needs to communicate some of the platform power changes down
> to the qemu code for things like shutdown/reset control.
>
> Space has been left to extend the EC if we find other use cases in
> future where ARM-TF and qemu need to communicate.
>
> Signed-off-by: Graeme Gregory
> ---
> hw/arm/sbsa-ref.c | 95 +++
> 1 file changed, 95 insertions(+)
>
> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> index f030a416fd..c8743fc1d0 100644
> --- a/hw/arm/sbsa-ref.c
> +++ b/hw/arm/sbsa-ref.c
> @@ -41,6 +41,7 @@
> #include "hw/usb.h"
> #include "hw/char/pl011.h"
> #include "net/net.h"
> +#include "migration/vmstate.h"
>
> #define RAMLIMIT_GB 8192
> #define RAMLIMIT_BYTES (RAMLIMIT_GB * GiB)
> @@ -62,6 +63,7 @@ enum {
> SBSA_CPUPERIPHS,
> SBSA_GIC_DIST,
> SBSA_GIC_REDIST,
> +SBSA_SECURE_EC,
> SBSA_SMMU,
> SBSA_UART,
> SBSA_RTC,
> @@ -98,6 +100,14 @@ typedef struct {
> #define SBSA_MACHINE(obj) \
> OBJECT_CHECK(SBSAMachineState, (obj), TYPE_SBSA_MACHINE)
>
> +typedef struct {
> +SysBusDevice parent_obj;
> +MemoryRegion iomem;
> +} SECUREECState;
> +
> +#define TYPE_SECURE_EC "sbsa-secure-ec"
> +#define SECURE_EC(obj) OBJECT_CHECK(SECUREECState, (obj), TYPE_SECURE_EC)
> +
> static const MemMapEntry sbsa_ref_memmap[] = {
> /* 512M boot ROM */
> [SBSA_FLASH] = { 0, 0x2000 },
> @@ -107,6 +117,7 @@ static const MemMapEntry sbsa_ref_memmap[] = {
> [SBSA_CPUPERIPHS] = { 0x4000, 0x0004 },
> [SBSA_GIC_DIST] = { 0x4006, 0x0001 },
> [SBSA_GIC_REDIST] = { 0x4008, 0x0400 },
> +[SBSA_SECURE_EC] = { 0x5000, 0x1000 },
> [SBSA_UART] = { 0x6000, 0x1000 },
> [SBSA_RTC] ={ 0x6001, 0x1000 },
> [SBSA_GPIO] = { 0x6002, 0x1000 },
> @@ -585,6 +596,65 @@ static void *sbsa_ref_dtb(const struct arm_boot_info
> *binfo, int *fdt_size)
> return board->fdt;
> }
>
> +enum sbsa_secure_ec_powerstates {
> +SBSA_SECURE_EC_CMD_NULL,
> +SBSA_SECURE_EC_CMD_POWEROFF,
> +SBSA_SECURE_EC_CMD_REBOOT,
> +};
> +
> +static uint64_t secure_ec_read(void *opaque, hwaddr offset, unsigned size)
> +{
> +/* No use for this currently */
> +return 0;
> +}
> +
> +static void secure_ec_write(void *opaque, hwaddr offset,
> + uint64_t value, unsigned size)
> +{
> +if (offset == 0) { /* PSCI machine power command register */
> +switch (value) {
> +case SBSA_SECURE_EC_CMD_NULL:
> +break;
> +case SBSA_SECURE_EC_CMD_POWEROFF:
> +qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
> +break;
> +case SBSA_SECURE_EC_CMD_REBOOT:
> +qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
> +break;
> +default:
> +error_report("sbsa-ref: ERROR Unkown power command");
> +}
> +} else {
> +error_report("sbsa-ref: unknown EC register");
> +}
> +}
> +
> +static const MemoryRegionOps secure_ec_ops = {
> +.read = secure_ec_read,
> +.write = secure_ec_write,
> +.endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static void secure_ec_init(Object *obj)
> +{
> +SECUREECState *s = SECURE_EC(obj);
> +SysBusDevice *dev = SYS_BUS_DEVICE(obj);
> +
> +memory_region_init_io(&s->iomem, obj, &secure_ec_ops, s, "secure-ec",
> +0x1000);
> +sysbus_init_mmio(dev, &s->iomem);
> +}
> +
> +static void create_secure_ec(MemoryRegion *mem)
> +{
> +hwaddr base = sbsa_ref_memmap[SBSA_SECURE_EC].base;
> +DeviceState *dev = qdev_create(NULL, TYPE_SECURE_EC);
I've just discovered the API change here, I tested in an older tree, but
Ill wait for other reviews before re-issuing.
Graeme
> +SysBusDevice *s = SYS_BUS_DEVICE(dev);
> +
> +memory_region_add_subregion(mem, base,
> +sysbus_mmio_get_region(s, 0));
> +}
> +
> static void sbsa_ref_init(MachineState *machine)
> {
> unsigned int smp_cpus = machine->smp.cpus;
> @@ -708,6 +778,8 @@ static void sbsa_ref_init(MachineState *machine)
>
> create_pcie(sms);
>
&
Re: [Qemu-devel] [PATCH] hw/arm/virt-acpi-build: _CCA attribute is compulsary
On Tue, 3 Nov 2015, at 02:25 AM, Shannon Zhao wrote:
> Hi Graeme,
>
> On 2015/11/2 18:39, Graeme Gregory wrote:
> > According to ACPI specification 6.2.17 _CCA (Cache Coherency Attribute)
> > this attribute is compulsary on ARM systems. Add this attribute to
> > the PCI host bridges as required.
> >
>
> To ACPI 5.1 this object is not compulsory and if not supplied it has
> default value for it. But to ACPI 6.0 it must be supplied on ARM systems.
> Regarding this change, ACPI 6.0 fixes 5.1 for this object, right?
>
Hi Shannon, the wording in ACPI 5.1 is "On ARM based systems, the _CCA
object must be supplied all such devices."
So is not functionally different from 6.0.
Graeme
> > Without this the kernel will produce the error
> > [Firmware Bug]: PCI device :00:00.0 fail to setup DMA.
> >
> > Signed-off-by: Graeme Gregory
> > ---
> > hw/arm/virt-acpi-build.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > index 1aaff1f..1430125 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -180,6 +180,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const
> > MemMapEntry *memmap, int irq,
> > aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
> > aml_append(dev, aml_name_decl("_UID", aml_string("PCI0")));
> > aml_append(dev, aml_name_decl("_STR", aml_unicode("PCIe 0 Device")));
> > +aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
> >
> > /* Declare the PCI Routing Table. */
> > Aml *rt_pkg = aml_package(nr_pcie_buses * PCI_NUM_PINS);
> >
>
> --
> Shannon
>
>
[Qemu-devel] [PATCH] hw/arm/virt-acpi-build: _CCA attribute is compulsary
According to ACPI specification 6.2.17 _CCA (Cache Coherency Attribute)
this attribute is compulsary on ARM systems. Add this attribute to
the PCI host bridges as required.
Without this the kernel will produce the error
[Firmware Bug]: PCI device :00:00.0 fail to setup DMA.
Signed-off-by: Graeme Gregory
---
hw/arm/virt-acpi-build.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 1aaff1f..1430125 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -180,6 +180,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry
*memmap, int irq,
aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
aml_append(dev, aml_name_decl("_UID", aml_string("PCI0")));
aml_append(dev, aml_name_decl("_STR", aml_unicode("PCIe 0 Device")));
+aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
/* Declare the PCI Routing Table. */
Aml *rt_pkg = aml_package(nr_pcie_buses * PCI_NUM_PINS);
--
2.4.3
Re: [Qemu-devel] [Linaro-acpi] [RFC PATCH 0/7] hw/arm/virt: Dynamic ACPI v5.1 table generation
On Wed, Nov 12, 2014 at 11:07:22AM +, Mark Rutland wrote: > [...] > > > > > > > We are currently suggesting adding an RDSP property to the chosen > > > > > > node > > > > > > in the tiny DT, but a command-line arguement like kexec proposed > > > > > > could > > > > > > be another option I guess, albeit not a very pretty one. > > > > > > > > > > I'm not sure what an RDSP command line property would have to do with > > > > > kexec. I'll assume I've misunderstood something. > > > > > > > > > > > > > I thought the kexec patches proposed passing the RDSP on the > > > > command-line to boot the secondary kernel, so if that ended up being > > > > supported by the kernel for kexec, maybe that could be leveraged by > > > > Xen's boot protocol. It was an idea someone brought to me, just thought > > > > I'd mention it. > > > > > > Ah, that's not something I'd heard of. > > > > Maybe there was a misunderstanding then, I thought you were cc'ed on > > those discussions. > > I may just have lost them in my inbox. I'm on a few too many mailing > lists these days. Sorry about that. > > > > I'm not a fan of placing fundamentally required system description on > > > the command line. It's fine for explicit overrides but I don't think it > > > should be the default mechanism as that causes its own set of problems > > > (who wants to fight with their hypervisor to pass a command line to a > > > guest kernel?). > > > > > > > Agreed completely, but I've been lacking strong technical arguments > > against passing this stuff on the cmdline. My personal preferred > > approach for the Xen Dom0 case is to add a property to the DT. > > Something under /chosen, or a firmware node would sound preferable to > me. For UEFI we pass the system table address as > /chosen/linux,uefi-system-table = <... ...>, and I think the RDSP could > be handled similarly if necessary. The user can than override that via > the command line if desired. > We have actually done that in the past when we had to support u-boot and bootwrapper as a bootloader. It works fine in the kernel and its a minimal patch to support. Graeme
