All of lore.kernel.org
 help / color / mirror / Atom feed
* [PULL 00/34] target-arm queue
@ 2020-07-03 16:53 Peter Maydell
  2020-07-03 16:53 ` [PULL 01/34] Add a phy-num property to the i.MX FEC emulator Peter Maydell
                   ` (35 more replies)
  0 siblings, 36 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:53 UTC (permalink / raw)
  To: qemu-devel

I might squeeze in another pullreq before softfreeze, but the
queue was already big enough that I wanted to send this lot out now.

-- PMM

The following changes since commit 4abf70a661a5df3886ac9d7c19c3617fa92b922a:

  Merge remote-tracking branch 'remotes/maxreitz/tags/pull-block-2020-06-24' into staging (2020-07-03 15:34:45 +0100)

are available in the Git repository at:

  https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20200703

for you to fetch changes up to 0f10bf84a9d489259a5b11c6aa1b05c1175b76ea:

  Deprecate TileGX port (2020-07-03 16:59:46 +0100)

----------------------------------------------------------------
target-arm queue:
 * i.MX6UL EVK board: put PHYs in the correct places
 * hw/arm/virt: Let the virtio-iommu bypass MSIs
 * target/arm: kvm: Handle DABT with no valid ISS
 * hw/arm/virt-acpi-build: Only expose flash on older machine types
 * target/arm: Fix temp double-free in sve ldr/str
 * hw/display/bcm2835_fb.c: Initialize all fields of struct
 * hw/arm/spitz: Code cleanup to fix Coverity-detected memory leak
 * Deprecate TileGX port

----------------------------------------------------------------
Andrew Jones (4):
      tests/acpi: remove stale allowed tables
      tests/acpi: virt: allow DSDT acpi table changes
      hw/arm/virt-acpi-build: Only expose flash on older machine types
      tests/acpi: virt: update golden masters for DSDT

Beata Michalska (2):
      target/arm: kvm: Handle DABT with no valid ISS
      target/arm: kvm: Handle misconfigured dabt injection

Eric Auger (5):
      qdev: Introduce DEFINE_PROP_RESERVED_REGION
      virtio-iommu: Implement RESV_MEM probe request
      virtio-iommu: Handle reserved regions in the translation process
      virtio-iommu-pci: Add array of Interval properties
      hw/arm/virt: Let the virtio-iommu bypass MSIs

Jean-Christophe Dubois (3):
      Add a phy-num property to the i.MX FEC emulator
      Add the ability to select a different PHY for each i.MX6UL FEC interface
      Select MDIO device 2 and 1 as PHY devices for i.MX6UL EVK board.

Peter Maydell (19):
      hw/display/bcm2835_fb.c: Initialize all fields of struct
      hw/arm/spitz: Detabify
      hw/arm/spitz: Create SpitzMachineClass abstract base class
      hw/arm/spitz: Keep pointers to MPU and SSI devices in SpitzMachineState
      hw/arm/spitz: Keep pointers to scp0, scp1 in SpitzMachineState
      hw/arm/spitz: Implement inbound GPIO lines for bit5 and power signals
      hw/misc/max111x: provide QOM properties for setting initial values
      hw/misc/max111x: Don't use vmstate_register()
      ssi: Add ssi_realize_and_unref()
      hw/arm/spitz: Use max111x properties to set initial values
      hw/misc/max111x: Use GPIO lines rather than max111x_set_input()
      hw/misc/max111x: Create header file for documentation, TYPE_ macros
      hw/arm/spitz: Encapsulate misc GPIO handling in a device
      hw/gpio/zaurus.c: Use LOG_GUEST_ERROR for bad guest register accesses
      hw/arm/spitz: Use LOG_GUEST_ERROR for bad guest register accesses
      hw/arm/pxa2xx_pic: Use LOG_GUEST_ERROR for bad guest register accesses
      hw/arm/spitz: Provide usual QOM macros for corgi-ssp and spitz-lcdtg
      Replace uses of FROM_SSI_SLAVE() macro with QOM casts
      Deprecate TileGX port

Richard Henderson (1):
      target/arm: Fix temp double-free in sve ldr/str

 docs/system/deprecated.rst                  |  11 +
 include/exec/memory.h                       |   6 +
 include/hw/arm/fsl-imx6ul.h                 |   2 +
 include/hw/arm/pxa.h                        |   1 -
 include/hw/arm/sharpsl.h                    |   3 -
 include/hw/arm/virt.h                       |   8 +
 include/hw/misc/max111x.h                   |  56 +++
 include/hw/net/imx_fec.h                    |   1 +
 include/hw/qdev-properties.h                |   3 +
 include/hw/ssi/ssi.h                        |  31 +-
 include/hw/virtio/virtio-iommu.h            |   2 +
 include/qemu/typedefs.h                     |   1 +
 target/arm/cpu.h                            |   2 +
 target/arm/kvm_arm.h                        |  10 +
 target/arm/translate-a64.h                  |   1 +
 tests/qtest/bios-tables-test-allowed-diff.h |  18 -
 hw/arm/fsl-imx6ul.c                         |  10 +
 hw/arm/mcimx6ul-evk.c                       |   2 +
 hw/arm/pxa2xx_pic.c                         |   9 +-
 hw/arm/spitz.c                              | 507 ++++++++++++++++------------
 hw/arm/virt-acpi-build.c                    |   5 +-
 hw/arm/virt.c                               |  33 ++
 hw/arm/z2.c                                 |  11 +-
 hw/core/qdev-properties.c                   |  89 +++++
 hw/display/ads7846.c                        |   9 +-
 hw/display/bcm2835_fb.c                     |   4 +
 hw/display/ssd0323.c                        |  10 +-
 hw/gpio/zaurus.c                            |  12 +-
 hw/misc/max111x.c                           |  86 +++--
 hw/net/imx_fec.c                            |  24 +-
 hw/sd/ssi-sd.c                              |   4 +-
 hw/ssi/ssi.c                                |   7 +-
 hw/virtio/virtio-iommu-pci.c                |  11 +
 hw/virtio/virtio-iommu.c                    | 114 ++++++-
 target/arm/kvm.c                            |  80 +++++
 target/arm/kvm32.c                          |  34 ++
 target/arm/kvm64.c                          |  49 +++
 target/arm/translate-a64.c                  |   6 +
 target/arm/translate-sve.c                  |   8 +-
 MAINTAINERS                                 |   1 +
 hw/net/trace-events                         |   4 +-
 hw/virtio/trace-events                      |   1 +
 tests/data/acpi/virt/DSDT                   | Bin 5307 -> 5205 bytes
 tests/data/acpi/virt/DSDT.memhp             | Bin 6668 -> 6566 bytes
 tests/data/acpi/virt/DSDT.numamem           | Bin 5307 -> 5205 bytes
 45 files changed, 974 insertions(+), 312 deletions(-)
 create mode 100644 include/hw/misc/max111x.h


^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PULL 01/34] Add a phy-num property to the i.MX FEC emulator
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
@ 2020-07-03 16:53 ` Peter Maydell
  2020-07-03 16:53 ` [PULL 02/34] Add the ability to select a different PHY for each i.MX6UL FEC interface Peter Maydell
                   ` (34 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:53 UTC (permalink / raw)
  To: qemu-devel

From: Jean-Christophe Dubois <jcd@tribudubois.net>

We need a solution to use an Ethernet PHY that is not the first device
on the MDIO bus (device 0 on MDIO bus).

As an example with the i.MX6UL the NXP SOC has 2 Ethernet devices but
only one MDIO bus on which the 2 related PHY are connected but at unique
addresses.

Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
Message-id: a1a5c0e139d1c763194b8020573dcb6025daeefa.1593296112.git.jcd@tribudubois.net
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/net/imx_fec.h |  1 +
 hw/net/imx_fec.c         | 24 +++++++++++++++++-------
 hw/net/trace-events      |  4 ++--
 3 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/include/hw/net/imx_fec.h b/include/hw/net/imx_fec.h
index 7b3faa40194..9f03034b893 100644
--- a/include/hw/net/imx_fec.h
+++ b/include/hw/net/imx_fec.h
@@ -268,6 +268,7 @@ typedef struct IMXFECState {
     uint32_t phy_advertise;
     uint32_t phy_int;
     uint32_t phy_int_mask;
+    uint32_t phy_num;
 
     bool is_fec;
 
diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
index eefedc252de..2c148040414 100644
--- a/hw/net/imx_fec.c
+++ b/hw/net/imx_fec.c
@@ -280,12 +280,16 @@ static void imx_phy_reset(IMXFECState *s)
 static uint32_t imx_phy_read(IMXFECState *s, int reg)
 {
     uint32_t val;
+    uint32_t phy = reg / 32;
 
-    if (reg > 31) {
-        /* we only advertise one phy */
+    if (phy != s->phy_num) {
+        qemu_log_mask(LOG_GUEST_ERROR, "[%s.phy]%s: Bad phy num %u\n",
+                      TYPE_IMX_FEC, __func__, phy);
         return 0;
     }
 
+    reg %= 32;
+
     switch (reg) {
     case 0:     /* Basic Control */
         val = s->phy_control;
@@ -331,20 +335,25 @@ static uint32_t imx_phy_read(IMXFECState *s, int reg)
         break;
     }
 
-    trace_imx_phy_read(val, reg);
+    trace_imx_phy_read(val, phy, reg);
 
     return val;
 }
 
 static void imx_phy_write(IMXFECState *s, int reg, uint32_t val)
 {
-    trace_imx_phy_write(val, reg);
+    uint32_t phy = reg / 32;
 
-    if (reg > 31) {
-        /* we only advertise one phy */
+    if (phy != s->phy_num) {
+        qemu_log_mask(LOG_GUEST_ERROR, "[%s.phy]%s: Bad phy num %u\n",
+                      TYPE_IMX_FEC, __func__, phy);
         return;
     }
 
+    reg %= 32;
+
+    trace_imx_phy_write(val, phy, reg);
+
     switch (reg) {
     case 0:     /* Basic Control */
         if (val & 0x8000) {
@@ -926,7 +935,7 @@ static void imx_eth_write(void *opaque, hwaddr offset, uint64_t value,
                                                        extract32(value,
                                                                  18, 10)));
         } else {
-            /* This a write operation */
+            /* This is a write operation */
             imx_phy_write(s, extract32(value, 18, 10), extract32(value, 0, 16));
         }
         /* raise the interrupt as the PHY operation is done */
@@ -1315,6 +1324,7 @@ static void imx_eth_realize(DeviceState *dev, Error **errp)
 static Property imx_eth_properties[] = {
     DEFINE_NIC_PROPERTIES(IMXFECState, conf),
     DEFINE_PROP_UINT32("tx-ring-num", IMXFECState, tx_ring_num, 1),
+    DEFINE_PROP_UINT32("phy-num", IMXFECState, phy_num, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/net/trace-events b/hw/net/trace-events
index e6875c4c0f6..5db45456d92 100644
--- a/hw/net/trace-events
+++ b/hw/net/trace-events
@@ -413,8 +413,8 @@ i82596_set_multicast(uint16_t count) "Added %d multicast entries"
 i82596_channel_attention(void *s) "%p: Received CHANNEL ATTENTION"
 
 # imx_fec.c
-imx_phy_read(uint32_t val, int reg) "0x%04"PRIx32" <= reg[%d]"
-imx_phy_write(uint32_t val, int reg) "0x%04"PRIx32" => reg[%d]"
+imx_phy_read(uint32_t val, int phy, int reg) "0x%04"PRIx32" <= phy[%d].reg[%d]"
+imx_phy_write(uint32_t val, int phy, int reg) "0x%04"PRIx32" => phy[%d].reg[%d]"
 imx_phy_update_link(const char *s) "%s"
 imx_phy_reset(void) ""
 imx_fec_read_bd(uint64_t addr, int flags, int len, int data) "tx_bd 0x%"PRIx64" flags 0x%04x len %d data 0x%08x"
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 02/34] Add the ability to select a different PHY for each i.MX6UL FEC interface
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
  2020-07-03 16:53 ` [PULL 01/34] Add a phy-num property to the i.MX FEC emulator Peter Maydell
@ 2020-07-03 16:53 ` Peter Maydell
  2020-07-03 16:53 ` [PULL 03/34] Select MDIO device 2 and 1 as PHY devices for i.MX6UL EVK board Peter Maydell
                   ` (33 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:53 UTC (permalink / raw)
  To: qemu-devel

From: Jean-Christophe Dubois <jcd@tribudubois.net>

Add properties to the i.MX6UL processor to be able to select a
particular PHY on the MDIO bus for each FEC device.

Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
Message-id: ea1d604198b6b73ea6521676e45bacfc597aba53.1593296112.git.jcd@tribudubois.net
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/fsl-imx6ul.h |  2 ++
 hw/arm/fsl-imx6ul.c         | 10 ++++++++++
 2 files changed, 12 insertions(+)

diff --git a/include/hw/arm/fsl-imx6ul.h b/include/hw/arm/fsl-imx6ul.h
index 37c89cc5f92..fcbaf3dc861 100644
--- a/include/hw/arm/fsl-imx6ul.h
+++ b/include/hw/arm/fsl-imx6ul.h
@@ -87,6 +87,8 @@ typedef struct FslIMX6ULState {
     MemoryRegion       caam;
     MemoryRegion       ocram;
     MemoryRegion       ocram_alias;
+
+    uint32_t           phy_num[FSL_IMX6UL_NUM_ETHS];
 } FslIMX6ULState;
 
 enum FslIMX6ULMemoryMap {
diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
index 6446034711e..51b2f256ec8 100644
--- a/hw/arm/fsl-imx6ul.c
+++ b/hw/arm/fsl-imx6ul.c
@@ -427,6 +427,9 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error **errp)
             FSL_IMX6UL_ENET2_TIMER_IRQ,
         };
 
+        object_property_set_uint(OBJECT(&s->eth[i]),
+                                 s->phy_num[i],
+                                 "phy-num", &error_abort);
         object_property_set_uint(OBJECT(&s->eth[i]),
                                  FSL_IMX6UL_ETH_NUM_TX_RINGS,
                                  "tx-ring-num", &error_abort);
@@ -607,10 +610,17 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error **errp)
                                 FSL_IMX6UL_OCRAM_ALIAS_ADDR, &s->ocram_alias);
 }
 
+static Property fsl_imx6ul_properties[] = {
+    DEFINE_PROP_UINT32("fec1-phy-num", FslIMX6ULState, phy_num[0], 0),
+    DEFINE_PROP_UINT32("fec2-phy-num", FslIMX6ULState, phy_num[1], 1),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void fsl_imx6ul_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
 
+    device_class_set_props(dc, fsl_imx6ul_properties);
     dc->realize = fsl_imx6ul_realize;
     dc->desc = "i.MX6UL SOC";
     /* Reason: Uses serial_hds and nd_table in realize() directly */
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 03/34] Select MDIO device 2 and 1 as PHY devices for i.MX6UL EVK board.
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
  2020-07-03 16:53 ` [PULL 01/34] Add a phy-num property to the i.MX FEC emulator Peter Maydell
  2020-07-03 16:53 ` [PULL 02/34] Add the ability to select a different PHY for each i.MX6UL FEC interface Peter Maydell
@ 2020-07-03 16:53 ` Peter Maydell
  2020-07-03 16:53 ` [PULL 04/34] qdev: Introduce DEFINE_PROP_RESERVED_REGION Peter Maydell
                   ` (32 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:53 UTC (permalink / raw)
  To: qemu-devel

From: Jean-Christophe Dubois <jcd@tribudubois.net>

The i.MX6UL EVK 14x14 board uses:
- PHY 2 for FEC 1
- PHY 1 for FEC 2

Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
Message-id: fb41992126c091a71d76ab3d1898959091f60583.1593296112.git.jcd@tribudubois.net
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/mcimx6ul-evk.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/arm/mcimx6ul-evk.c b/hw/arm/mcimx6ul-evk.c
index 2f845cedfce..9033d3f8f38 100644
--- a/hw/arm/mcimx6ul-evk.c
+++ b/hw/arm/mcimx6ul-evk.c
@@ -40,6 +40,8 @@ static void mcimx6ul_evk_init(MachineState *machine)
 
     s = FSL_IMX6UL(object_new(TYPE_FSL_IMX6UL));
     object_property_add_child(OBJECT(machine), "soc", OBJECT(s));
+    object_property_set_uint(OBJECT(s), 2, "fec1-phy-num", &error_fatal);
+    object_property_set_uint(OBJECT(s), 1, "fec2-phy-num", &error_fatal);
     qdev_realize(DEVICE(s), NULL, &error_fatal);
 
     memory_region_add_subregion(get_system_memory(), FSL_IMX6UL_MMDC_ADDR,
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 04/34] qdev: Introduce DEFINE_PROP_RESERVED_REGION
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (2 preceding siblings ...)
  2020-07-03 16:53 ` [PULL 03/34] Select MDIO device 2 and 1 as PHY devices for i.MX6UL EVK board Peter Maydell
@ 2020-07-03 16:53 ` Peter Maydell
  2020-07-03 16:53 ` [PULL 05/34] virtio-iommu: Implement RESV_MEM probe request Peter Maydell
                   ` (31 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:53 UTC (permalink / raw)
  To: qemu-devel

From: Eric Auger <eric.auger@redhat.com>

Introduce a new property defining a reserved region:
<low address>:<high address>:<type>.

This will be used to encode reserved IOVA regions.

For instance, in virtio-iommu use case, reserved IOVA regions
will be passed by the machine code to the virtio-iommu-pci
device (an array of those). The type of the reserved region
will match the virtio_iommu_probe_resv_mem subtype value:
- VIRTIO_IOMMU_RESV_MEM_T_RESERVED (0)
- VIRTIO_IOMMU_RESV_MEM_T_MSI (1)

on PC/Q35 machine, this will be used to inform the
virtio-iommu-pci device it should bypass the MSI region.
The reserved region will be: 0xfee00000:0xfeefffff:1.

On ARM, we can declare the ITS MSI doorbell as an MSI
region to prevent MSIs from being mapped on guest side.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-id: 20200629070404.10969-2-eric.auger@redhat.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/exec/memory.h        |  6 +++
 include/hw/qdev-properties.h |  3 ++
 include/qemu/typedefs.h      |  1 +
 hw/core/qdev-properties.c    | 89 ++++++++++++++++++++++++++++++++++++
 4 files changed, 99 insertions(+)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 7207025bd49..84ee5b7a01f 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -51,6 +51,12 @@ extern bool global_dirty_log;
 
 typedef struct MemoryRegionOps MemoryRegionOps;
 
+struct ReservedRegion {
+    hwaddr low;
+    hwaddr high;
+    unsigned type;
+};
+
 typedef struct IOMMUTLBEntry IOMMUTLBEntry;
 
 /* See address_space_translate: bit 0 is read, bit 1 is write.  */
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 49c6cd24607..944e3f2e0cd 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -19,6 +19,7 @@ extern const PropertyInfo qdev_prop_string;
 extern const PropertyInfo qdev_prop_chr;
 extern const PropertyInfo qdev_prop_tpm;
 extern const PropertyInfo qdev_prop_macaddr;
+extern const PropertyInfo qdev_prop_reserved_region;
 extern const PropertyInfo qdev_prop_on_off_auto;
 extern const PropertyInfo qdev_prop_multifd_compression;
 extern const PropertyInfo qdev_prop_losttickpolicy;
@@ -184,6 +185,8 @@ extern const PropertyInfo qdev_prop_pcie_link_width;
     DEFINE_PROP(_n, _s, _f, qdev_prop_drive_iothread, BlockBackend *)
 #define DEFINE_PROP_MACADDR(_n, _s, _f)         \
     DEFINE_PROP(_n, _s, _f, qdev_prop_macaddr, MACAddr)
+#define DEFINE_PROP_RESERVED_REGION(_n, _s, _f)         \
+    DEFINE_PROP(_n, _s, _f, qdev_prop_reserved_region, ReservedRegion)
 #define DEFINE_PROP_ON_OFF_AUTO(_n, _s, _f, _d) \
     DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_on_off_auto, OnOffAuto)
 #define DEFINE_PROP_MULTIFD_COMPRESSION(_n, _s, _f, _d) \
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index ce4a78b687a..15f5047bf1d 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -58,6 +58,7 @@ typedef struct ISABus ISABus;
 typedef struct ISADevice ISADevice;
 typedef struct IsaDma IsaDma;
 typedef struct MACAddr MACAddr;
+typedef struct ReservedRegion ReservedRegion;
 typedef struct MachineClass MachineClass;
 typedef struct MachineState MachineState;
 typedef struct MemoryListener MemoryListener;
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 71f8aca7c60..ca7771f3072 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -15,6 +15,7 @@
 #include "chardev/char.h"
 #include "qemu/uuid.h"
 #include "qemu/units.h"
+#include "qemu/cutils.h"
 
 void qdev_prop_set_after_realize(DeviceState *dev, const char *name,
                                   Error **errp)
@@ -578,6 +579,94 @@ const PropertyInfo qdev_prop_macaddr = {
     .set   = set_mac,
 };
 
+/* --- Reserved Region --- */
+
+/*
+ * Accepted syntax:
+ *   <low address>:<high address>:<type>
+ *   where low/high addresses are uint64_t in hexadecimal
+ *   and type is a non-negative decimal integer
+ */
+static void get_reserved_region(Object *obj, Visitor *v, const char *name,
+                                void *opaque, Error **errp)
+{
+    DeviceState *dev = DEVICE(obj);
+    Property *prop = opaque;
+    ReservedRegion *rr = qdev_get_prop_ptr(dev, prop);
+    char buffer[64];
+    char *p = buffer;
+    int rc;
+
+    rc = snprintf(buffer, sizeof(buffer), "0x%"PRIx64":0x%"PRIx64":%u",
+                  rr->low, rr->high, rr->type);
+    assert(rc < sizeof(buffer));
+
+    visit_type_str(v, name, &p, errp);
+}
+
+static void set_reserved_region(Object *obj, Visitor *v, const char *name,
+                                void *opaque, Error **errp)
+{
+    DeviceState *dev = DEVICE(obj);
+    Property *prop = opaque;
+    ReservedRegion *rr = qdev_get_prop_ptr(dev, prop);
+    Error *local_err = NULL;
+    const char *endptr;
+    char *str;
+    int ret;
+
+    if (dev->realized) {
+        qdev_prop_set_after_realize(dev, name, errp);
+        return;
+    }
+
+    visit_type_str(v, name, &str, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    ret = qemu_strtou64(str, &endptr, 16, &rr->low);
+    if (ret) {
+        error_setg(errp, "start address of '%s'"
+                   " must be a hexadecimal integer", name);
+        goto out;
+    }
+    if (*endptr != ':') {
+        goto separator_error;
+    }
+
+    ret = qemu_strtou64(endptr + 1, &endptr, 16, &rr->high);
+    if (ret) {
+        error_setg(errp, "end address of '%s'"
+                   " must be a hexadecimal integer", name);
+        goto out;
+    }
+    if (*endptr != ':') {
+        goto separator_error;
+    }
+
+    ret = qemu_strtoui(endptr + 1, &endptr, 10, &rr->type);
+    if (ret) {
+        error_setg(errp, "type of '%s'"
+                   " must be a non-negative decimal integer", name);
+    }
+    goto out;
+
+separator_error:
+    error_setg(errp, "reserved region fields must be separated with ':'");
+out:
+    g_free(str);
+    return;
+}
+
+const PropertyInfo qdev_prop_reserved_region = {
+    .name  = "reserved_region",
+    .description = "Reserved Region, example: 0xFEE00000:0xFEEFFFFF:0",
+    .get   = get_reserved_region,
+    .set   = set_reserved_region,
+};
+
 /* --- on/off/auto --- */
 
 const PropertyInfo qdev_prop_on_off_auto = {
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 05/34] virtio-iommu: Implement RESV_MEM probe request
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (3 preceding siblings ...)
  2020-07-03 16:53 ` [PULL 04/34] qdev: Introduce DEFINE_PROP_RESERVED_REGION Peter Maydell
@ 2020-07-03 16:53 ` Peter Maydell
  2020-07-05 18:21   ` Peter Maydell
  2020-07-03 16:53 ` [PULL 06/34] virtio-iommu: Handle reserved regions in the translation process Peter Maydell
                   ` (30 subsequent siblings)
  35 siblings, 1 reply; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:53 UTC (permalink / raw)
  To: qemu-devel

From: Eric Auger <eric.auger@redhat.com>

This patch implements the PROBE request. At the moment,
only THE RESV_MEM property is handled. The first goal is
to report iommu wide reserved regions such as the MSI regions
set by the machine code. On x86 this will be the IOAPIC MSI
region, [0xFEE00000 - 0xFEEFFFFF], on ARM this may be the ITS
doorbell.

In the future we may introduce per device reserved regions.
This will be useful when protecting host assigned devices
which may expose their own reserved regions

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-id: 20200629070404.10969-3-eric.auger@redhat.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/virtio/virtio-iommu.h |  2 +
 hw/virtio/virtio-iommu.c         | 94 ++++++++++++++++++++++++++++++--
 hw/virtio/trace-events           |  1 +
 3 files changed, 93 insertions(+), 4 deletions(-)

diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
index e653004d7ca..49eb105cd84 100644
--- a/include/hw/virtio/virtio-iommu.h
+++ b/include/hw/virtio/virtio-iommu.h
@@ -53,6 +53,8 @@ typedef struct VirtIOIOMMU {
     GHashTable *as_by_busptr;
     IOMMUPciBus *iommu_pcibus_by_bus_num[PCI_BUS_MAX];
     PCIBus *primary_bus;
+    ReservedRegion *reserved_regions;
+    uint32_t nb_reserved_regions;
     GTree *domains;
     QemuMutex mutex;
     GTree *endpoints;
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 483883ec1d9..2cdaa1969bb 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -38,6 +38,7 @@
 
 /* Max size */
 #define VIOMMU_DEFAULT_QUEUE_SIZE 256
+#define VIOMMU_PROBE_SIZE 512
 
 typedef struct VirtIOIOMMUDomain {
     uint32_t id;
@@ -378,6 +379,65 @@ static int virtio_iommu_unmap(VirtIOIOMMU *s,
     return ret;
 }
 
+static ssize_t virtio_iommu_fill_resv_mem_prop(VirtIOIOMMU *s, uint32_t ep,
+                                               uint8_t *buf, size_t free)
+{
+    struct virtio_iommu_probe_resv_mem prop = {};
+    size_t size = sizeof(prop), length = size - sizeof(prop.head), total;
+    int i;
+
+    total = size * s->nb_reserved_regions;
+
+    if (total > free) {
+        return -ENOSPC;
+    }
+
+    for (i = 0; i < s->nb_reserved_regions; i++) {
+        unsigned subtype = s->reserved_regions[i].type;
+
+        assert(subtype == VIRTIO_IOMMU_RESV_MEM_T_RESERVED ||
+               subtype == VIRTIO_IOMMU_RESV_MEM_T_MSI);
+        prop.head.type = cpu_to_le16(VIRTIO_IOMMU_PROBE_T_RESV_MEM);
+        prop.head.length = cpu_to_le16(length);
+        prop.subtype = subtype;
+        prop.start = cpu_to_le64(s->reserved_regions[i].low);
+        prop.end = cpu_to_le64(s->reserved_regions[i].high);
+
+        memcpy(buf, &prop, size);
+
+        trace_virtio_iommu_fill_resv_property(ep, prop.subtype,
+                                              prop.start, prop.end);
+        buf += size;
+    }
+    return total;
+}
+
+/**
+ * virtio_iommu_probe - Fill the probe request buffer with
+ * the properties the device is able to return
+ */
+static int virtio_iommu_probe(VirtIOIOMMU *s,
+                              struct virtio_iommu_req_probe *req,
+                              uint8_t *buf)
+{
+    uint32_t ep_id = le32_to_cpu(req->endpoint);
+    size_t free = VIOMMU_PROBE_SIZE;
+    ssize_t count;
+
+    if (!virtio_iommu_mr(s, ep_id)) {
+        return VIRTIO_IOMMU_S_NOENT;
+    }
+
+    count = virtio_iommu_fill_resv_mem_prop(s, ep_id, buf, free);
+    if (count < 0) {
+        return VIRTIO_IOMMU_S_INVAL;
+    }
+    buf += count;
+    free -= count;
+
+    return VIRTIO_IOMMU_S_OK;
+}
+
 static int virtio_iommu_iov_to_req(struct iovec *iov,
                                    unsigned int iov_cnt,
                                    void *req, size_t req_sz)
@@ -407,15 +467,27 @@ virtio_iommu_handle_req(detach)
 virtio_iommu_handle_req(map)
 virtio_iommu_handle_req(unmap)
 
+static int virtio_iommu_handle_probe(VirtIOIOMMU *s,
+                                     struct iovec *iov,
+                                     unsigned int iov_cnt,
+                                     uint8_t *buf)
+{
+    struct virtio_iommu_req_probe req;
+    int ret = virtio_iommu_iov_to_req(iov, iov_cnt, &req, sizeof(req));
+
+    return ret ? ret : virtio_iommu_probe(s, &req, buf);
+}
+
 static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
 {
     VirtIOIOMMU *s = VIRTIO_IOMMU(vdev);
     struct virtio_iommu_req_head head;
     struct virtio_iommu_req_tail tail = {};
+    size_t output_size = sizeof(tail), sz;
     VirtQueueElement *elem;
     unsigned int iov_cnt;
     struct iovec *iov;
-    size_t sz;
+    void *buf = NULL;
 
     for (;;) {
         elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
@@ -452,6 +524,17 @@ static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
         case VIRTIO_IOMMU_T_UNMAP:
             tail.status = virtio_iommu_handle_unmap(s, iov, iov_cnt);
             break;
+        case VIRTIO_IOMMU_T_PROBE:
+        {
+            struct virtio_iommu_req_tail *ptail;
+
+            output_size = s->config.probe_size + sizeof(tail);
+            buf = g_malloc0(output_size);
+
+            ptail = (struct virtio_iommu_req_tail *)
+                        (buf + s->config.probe_size);
+            ptail->status = virtio_iommu_handle_probe(s, iov, iov_cnt, buf);
+        }
         default:
             tail.status = VIRTIO_IOMMU_S_UNSUPP;
         }
@@ -459,12 +542,13 @@ static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
 
 out:
         sz = iov_from_buf(elem->in_sg, elem->in_num, 0,
-                          &tail, sizeof(tail));
-        assert(sz == sizeof(tail));
+                          buf ? buf : &tail, output_size);
+        assert(sz == output_size);
 
-        virtqueue_push(vq, elem, sizeof(tail));
+        virtqueue_push(vq, elem, sz);
         virtio_notify(vdev, vq);
         g_free(elem);
+        g_free(buf);
     }
 }
 
@@ -667,6 +751,7 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
     s->config.page_size_mask = TARGET_PAGE_MASK;
     s->config.input_range.end = -1UL;
     s->config.domain_range.end = 32;
+    s->config.probe_size = VIOMMU_PROBE_SIZE;
 
     virtio_add_feature(&s->features, VIRTIO_RING_F_EVENT_IDX);
     virtio_add_feature(&s->features, VIRTIO_RING_F_INDIRECT_DESC);
@@ -676,6 +761,7 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
     virtio_add_feature(&s->features, VIRTIO_IOMMU_F_MAP_UNMAP);
     virtio_add_feature(&s->features, VIRTIO_IOMMU_F_BYPASS);
     virtio_add_feature(&s->features, VIRTIO_IOMMU_F_MMIO);
+    virtio_add_feature(&s->features, VIRTIO_IOMMU_F_PROBE);
 
     qemu_mutex_init(&s->mutex);
 
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 6427a0047df..23109f69bbf 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -74,3 +74,4 @@ virtio_iommu_get_domain(uint32_t domain_id) "Alloc domain=%d"
 virtio_iommu_put_domain(uint32_t domain_id) "Free domain=%d"
 virtio_iommu_translate_out(uint64_t virt_addr, uint64_t phys_addr, uint32_t sid) "0x%"PRIx64" -> 0x%"PRIx64 " for sid=%d"
 virtio_iommu_report_fault(uint8_t reason, uint32_t flags, uint32_t endpoint, uint64_t addr) "FAULT reason=%d flags=%d endpoint=%d address =0x%"PRIx64
+virtio_iommu_fill_resv_property(uint32_t devid, uint8_t subtype, uint64_t start, uint64_t end) "dev= %d, type=%d start=0x%"PRIx64" end=0x%"PRIx64
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 06/34] virtio-iommu: Handle reserved regions in the translation process
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (4 preceding siblings ...)
  2020-07-03 16:53 ` [PULL 05/34] virtio-iommu: Implement RESV_MEM probe request Peter Maydell
@ 2020-07-03 16:53 ` Peter Maydell
  2020-07-03 16:53 ` [PULL 07/34] virtio-iommu-pci: Add array of Interval properties Peter Maydell
                   ` (29 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:53 UTC (permalink / raw)
  To: qemu-devel

From: Eric Auger <eric.auger@redhat.com>

When translating an address we need to check if it belongs to
a reserved virtual address range. If it does, there are 2 cases:

- it belongs to a RESERVED region: the guest should neither use
  this address in a MAP not instruct the end-point to DMA on
  them. We report an error

- It belongs to an MSI region: we bypass the translation.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-id: 20200629070404.10969-4-eric.auger@redhat.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/virtio/virtio-iommu.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 2cdaa1969bb..b39e836181e 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -607,6 +607,7 @@ static IOMMUTLBEntry virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr,
     uint32_t sid, flags;
     bool bypass_allowed;
     bool found;
+    int i;
 
     interval.low = addr;
     interval.high = addr + 1;
@@ -640,6 +641,25 @@ static IOMMUTLBEntry virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr,
         goto unlock;
     }
 
+    for (i = 0; i < s->nb_reserved_regions; i++) {
+        ReservedRegion *reg = &s->reserved_regions[i];
+
+        if (addr >= reg->low && addr <= reg->high) {
+            switch (reg->type) {
+            case VIRTIO_IOMMU_RESV_MEM_T_MSI:
+                entry.perm = flag;
+                break;
+            case VIRTIO_IOMMU_RESV_MEM_T_RESERVED:
+            default:
+                virtio_iommu_report_fault(s, VIRTIO_IOMMU_FAULT_R_MAPPING,
+                                          VIRTIO_IOMMU_FAULT_F_ADDRESS,
+                                          sid, addr);
+                break;
+            }
+            goto unlock;
+        }
+    }
+
     if (!ep->domain) {
         if (!bypass_allowed) {
             error_report_once("%s %02x:%02x.%01x not attached to any domain",
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 07/34] virtio-iommu-pci: Add array of Interval properties
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (5 preceding siblings ...)
  2020-07-03 16:53 ` [PULL 06/34] virtio-iommu: Handle reserved regions in the translation process Peter Maydell
@ 2020-07-03 16:53 ` Peter Maydell
  2020-07-03 16:53 ` [PULL 08/34] hw/arm/virt: Let the virtio-iommu bypass MSIs Peter Maydell
                   ` (28 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:53 UTC (permalink / raw)
  To: qemu-devel

From: Eric Auger <eric.auger@redhat.com>

The machine may need to pass reserved regions to the
virtio-iommu-pci device (such as the MSI window on x86
or the MSI doorbells on ARM).

So let's add an array of Interval properties.

Note: if some reserved regions are already set by the
machine code - which should be the case in general -,
the length of the property array is already set and
prevents the end-user from modifying them. For example,
attempting to use:

-device virtio-iommu-pci,\
 len-reserved-regions=1,reserved-regions[0]=0xfee00000:0xfeefffff:1

would result in the following error message:

qemu-system-aarch64: -device virtio-iommu-pci,addr=0xa,
len-reserved-regions=1,reserved-regions[0]=0xfee00000:0xfeefffff:1:
array size property len-reserved-regions may not be set more than once

Otherwise, for example, adding two reserved regions is achieved
using the following options:

-device virtio-iommu-pci,addr=0xa,len-reserved-regions=2,\
 reserved-regions[0]=0xfee00000:0xfeefffff:1,\
 reserved-regions[1]=0x1000000:100ffff:1

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Reviewed-by: Peter Xu <peterx@redhat.com>
Message-id: 20200629070404.10969-5-eric.auger@redhat.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/virtio/virtio-iommu-pci.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/hw/virtio/virtio-iommu-pci.c b/hw/virtio/virtio-iommu-pci.c
index 4588361d6b3..592abc9279c 100644
--- a/hw/virtio/virtio-iommu-pci.c
+++ b/hw/virtio/virtio-iommu-pci.c
@@ -33,6 +33,9 @@ struct VirtIOIOMMUPCI {
 
 static Property virtio_iommu_pci_properties[] = {
     DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0),
+    DEFINE_PROP_ARRAY("reserved-regions", VirtIOIOMMUPCI,
+                      vdev.nb_reserved_regions, vdev.reserved_regions,
+                      qdev_prop_reserved_region, ReservedRegion),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -40,6 +43,7 @@ static void virtio_iommu_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
 {
     VirtIOIOMMUPCI *dev = VIRTIO_IOMMU_PCI(vpci_dev);
     DeviceState *vdev = DEVICE(&dev->vdev);
+    VirtIOIOMMU *s = VIRTIO_IOMMU(vdev);
 
     if (!qdev_get_machine_hotplug_handler(DEVICE(vpci_dev))) {
         MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
@@ -54,6 +58,13 @@ static void virtio_iommu_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
                           "-no-acpi\n");
         return;
     }
+    for (int i = 0; i < s->nb_reserved_regions; i++) {
+        if (s->reserved_regions[i].type != VIRTIO_IOMMU_RESV_MEM_T_RESERVED &&
+            s->reserved_regions[i].type != VIRTIO_IOMMU_RESV_MEM_T_MSI) {
+            error_setg(errp, "reserved region %d has an invalid type", i);
+            error_append_hint(errp, "Valid values are 0 and 1\n");
+        }
+    }
     object_property_set_link(OBJECT(dev),
                              OBJECT(pci_get_bus(&vpci_dev->pci_dev)),
                              "primary-bus", &error_abort);
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 08/34] hw/arm/virt: Let the virtio-iommu bypass MSIs
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (6 preceding siblings ...)
  2020-07-03 16:53 ` [PULL 07/34] virtio-iommu-pci: Add array of Interval properties Peter Maydell
@ 2020-07-03 16:53 ` Peter Maydell
  2023-02-02 10:47   ` Philippe Mathieu-Daudé
  2020-07-03 16:53 ` [PULL 09/34] target/arm: kvm: Handle DABT with no valid ISS Peter Maydell
                   ` (27 subsequent siblings)
  35 siblings, 1 reply; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:53 UTC (permalink / raw)
  To: qemu-devel

From: Eric Auger <eric.auger@redhat.com>

At the moment the virtio-iommu translates MSI transactions.
This behavior is inherited from ARM SMMU. The virt machine
code knows where the guest MSI doorbells are so we can easily
declare those regions as VIRTIO_IOMMU_RESV_MEM_T_MSI. With that
setting the guest will not map MSIs through the IOMMU and those
transactions will be simply bypassed.

Depending on which MSI controller is in use (ITS or GICV2M),
we declare either:
- the ITS interrupt translation space (ITS_base + 0x10000),
  containing the GITS_TRANSLATOR or
- The GICV2M single frame, containing the MSI_SETSP_NS register.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Message-id: 20200629070404.10969-6-eric.auger@redhat.com
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/virt.h |  7 +++++++
 hw/arm/virt.c         | 30 ++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 31878ddc722..a18b6b397b6 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -96,6 +96,12 @@ typedef enum VirtIOMMUType {
     VIRT_IOMMU_VIRTIO,
 } VirtIOMMUType;
 
+typedef enum VirtMSIControllerType {
+    VIRT_MSI_CTRL_NONE,
+    VIRT_MSI_CTRL_GICV2M,
+    VIRT_MSI_CTRL_ITS,
+} VirtMSIControllerType;
+
 typedef enum VirtGICType {
     VIRT_GIC_VERSION_MAX,
     VIRT_GIC_VERSION_HOST,
@@ -136,6 +142,7 @@ typedef struct {
     OnOffAuto acpi;
     VirtGICType gic_version;
     VirtIOMMUType iommu;
+    VirtMSIControllerType msi_controller;
     uint16_t virtio_iommu_bdf;
     struct arm_boot_info bootinfo;
     MemMapEntry *memmap;
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index af3050bc4bd..7922f3c23a5 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -600,6 +600,7 @@ static void create_its(VirtMachineState *vms)
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, vms->memmap[VIRT_GIC_ITS].base);
 
     fdt_add_its_gic_node(vms);
+    vms->msi_controller = VIRT_MSI_CTRL_ITS;
 }
 
 static void create_v2m(VirtMachineState *vms)
@@ -620,6 +621,7 @@ static void create_v2m(VirtMachineState *vms)
     }
 
     fdt_add_v2m_gic_node(vms);
+    vms->msi_controller = VIRT_MSI_CTRL_GICV2M;
 }
 
 static void create_gic(VirtMachineState *vms)
@@ -2198,8 +2200,36 @@ out:
 static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
                                             DeviceState *dev, Error **errp)
 {
+    VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
+
     if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
         virt_memory_pre_plug(hotplug_dev, dev, errp);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
+        hwaddr db_start = 0, db_end = 0;
+        char *resv_prop_str;
+
+        switch (vms->msi_controller) {
+        case VIRT_MSI_CTRL_NONE:
+            return;
+        case VIRT_MSI_CTRL_ITS:
+            /* GITS_TRANSLATER page */
+            db_start = base_memmap[VIRT_GIC_ITS].base + 0x10000;
+            db_end = base_memmap[VIRT_GIC_ITS].base +
+                     base_memmap[VIRT_GIC_ITS].size - 1;
+            break;
+        case VIRT_MSI_CTRL_GICV2M:
+            /* MSI_SETSPI_NS page */
+            db_start = base_memmap[VIRT_GIC_V2M].base;
+            db_end = db_start + base_memmap[VIRT_GIC_V2M].size - 1;
+            break;
+        }
+        resv_prop_str = g_strdup_printf("0x%"PRIx64":0x%"PRIx64":%u",
+                                        db_start, db_end,
+                                        VIRTIO_IOMMU_RESV_MEM_T_MSI);
+
+        qdev_prop_set_uint32(dev, "len-reserved-regions", 1);
+        qdev_prop_set_string(dev, "reserved-regions[0]", resv_prop_str);
+        g_free(resv_prop_str);
     }
 }
 
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 09/34] target/arm: kvm: Handle DABT with no valid ISS
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (7 preceding siblings ...)
  2020-07-03 16:53 ` [PULL 08/34] hw/arm/virt: Let the virtio-iommu bypass MSIs Peter Maydell
@ 2020-07-03 16:53 ` Peter Maydell
  2020-07-03 16:53 ` [PULL 10/34] target/arm: kvm: Handle misconfigured dabt injection Peter Maydell
                   ` (26 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:53 UTC (permalink / raw)
  To: qemu-devel

From: Beata Michalska <beata.michalska@linaro.org>

On ARMv7 & ARMv8 some load/store instructions might trigger a data abort
exception with no valid ISS info to be decoded. The lack of decode info
makes it at least tricky to emulate those instruction which is one of the
(many) reasons why KVM will not even try to do so.

Add support for handling those by requesting KVM to inject external
dabt into the quest.

Signed-off-by: Beata Michalska <beata.michalska@linaro.org>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Message-id: 20200629114110.30723-2-beata.michalska@linaro.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/kvm.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 7c672c78b88..3a46f54f1fd 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -39,6 +39,7 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
 
 static bool cap_has_mp_state;
 static bool cap_has_inject_serror_esr;
+static bool cap_has_inject_ext_dabt;
 
 static ARMHostCPUFeatures arm_host_cpu_features;
 
@@ -245,6 +246,16 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
         ret = -EINVAL;
     }
 
+    if (kvm_check_extension(s, KVM_CAP_ARM_NISV_TO_USER)) {
+        if (kvm_vm_enable_cap(s, KVM_CAP_ARM_NISV_TO_USER, 0)) {
+            error_report("Failed to enable KVM_CAP_ARM_NISV_TO_USER cap");
+        } else {
+            /* Set status for supporting the external dabt injection */
+            cap_has_inject_ext_dabt = kvm_check_extension(s,
+                                    KVM_CAP_ARM_INJECT_EXT_DABT);
+        }
+    }
+
     return ret;
 }
 
@@ -810,6 +821,42 @@ void kvm_arm_vm_state_change(void *opaque, int running, RunState state)
     }
 }
 
+/**
+ * kvm_arm_handle_dabt_nisv:
+ * @cs: CPUState
+ * @esr_iss: ISS encoding (limited) for the exception from Data Abort
+ *           ISV bit set to '0b0' -> no valid instruction syndrome
+ * @fault_ipa: faulting address for the synchronous data abort
+ *
+ * Returns: 0 if the exception has been handled, < 0 otherwise
+ */
+static int kvm_arm_handle_dabt_nisv(CPUState *cs, uint64_t esr_iss,
+                                    uint64_t fault_ipa)
+{
+    /*
+     * Request KVM to inject the external data abort into the guest
+     */
+    if (cap_has_inject_ext_dabt) {
+        struct kvm_vcpu_events events = { };
+        /*
+         * The external data abort event will be handled immediately by KVM
+         * using the address fault that triggered the exit on given VCPU.
+         * Requesting injection of the external data abort does not rely
+         * on any other VCPU state. Therefore, in this particular case, the VCPU
+         * synchronization can be exceptionally skipped.
+         */
+        events.exception.ext_dabt_pending = 1;
+        /* KVM_CAP_ARM_INJECT_EXT_DABT implies KVM_CAP_VCPU_EVENTS */
+        return kvm_vcpu_ioctl(cs, KVM_SET_VCPU_EVENTS, &events);
+    } else {
+        error_report("Data abort exception triggered by guest memory access "
+                     "at physical address: 0x"  TARGET_FMT_lx,
+                     (target_ulong)fault_ipa);
+        error_printf("KVM unable to emulate faulting instruction.\n");
+    }
+    return -1;
+}
+
 int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
 {
     int ret = 0;
@@ -820,6 +867,11 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
             ret = EXCP_DEBUG;
         } /* otherwise return to guest */
         break;
+    case KVM_EXIT_ARM_NISV:
+        /* External DABT with no valid iss to decode */
+        ret = kvm_arm_handle_dabt_nisv(cs, run->arm_nisv.esr_iss,
+                                       run->arm_nisv.fault_ipa);
+        break;
     default:
         qemu_log_mask(LOG_UNIMP, "%s: un-handled exit reason %d\n",
                       __func__, run->exit_reason);
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 10/34] target/arm: kvm: Handle misconfigured dabt injection
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (8 preceding siblings ...)
  2020-07-03 16:53 ` [PULL 09/34] target/arm: kvm: Handle DABT with no valid ISS Peter Maydell
@ 2020-07-03 16:53 ` Peter Maydell
  2020-07-03 16:53 ` [PULL 11/34] tests/acpi: remove stale allowed tables Peter Maydell
                   ` (25 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:53 UTC (permalink / raw)
  To: qemu-devel

From: Beata Michalska <beata.michalska@linaro.org>

Injecting external data abort through KVM might trigger
an issue on kernels that do not get updated to include the KVM fix.
For those and aarch32 guests, the injected abort gets misconfigured
to be an implementation defined exception. This leads to the guest
repeatedly re-running the faulting instruction.

Add support for handling that case.

[
  Fixed-by: 018f22f95e8a
	('KVM: arm: Fix DFSR setting for non-LPAE aarch32 guests')
  Fixed-by: 21aecdbd7f3a
	('KVM: arm: Make inject_abt32() inject an external abort instead')
]

Signed-off-by: Beata Michalska <beata.michalska@linaro.org>
Acked-by: Andrew Jones <drjones@redhat.com>
Message-id: 20200629114110.30723-3-beata.michalska@linaro.org
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/cpu.h     |  2 ++
 target/arm/kvm_arm.h | 10 +++++++++
 target/arm/kvm.c     | 30 ++++++++++++++++++++++++++-
 target/arm/kvm32.c   | 34 ++++++++++++++++++++++++++++++
 target/arm/kvm64.c   | 49 ++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 124 insertions(+), 1 deletion(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index cf99dcca9f3..9e8ed423ea1 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -573,6 +573,8 @@ typedef struct CPUARMState {
         uint64_t esr;
     } serror;
 
+    uint8_t ext_dabt_raised; /* Tracking/verifying injection of ext DABT */
+
     /* State of our input IRQ/FIQ/VIRQ/VFIQ lines */
     uint32_t irq_line_state;
 
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index a4ce4fd93db..adb38514bf2 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -449,6 +449,16 @@ bool kvm_arm_hw_debug_active(CPUState *cs);
 struct kvm_guest_debug_arch;
 void kvm_arm_copy_hw_debug_data(struct kvm_guest_debug_arch *ptr);
 
+/**
+ * kvm_arm_verify_ext_dabt_pending:
+ * @cs: CPUState
+ *
+ * Verify the fault status code wrt the Ext DABT injection
+ *
+ * Returns: true if the fault status code is as expected, false otherwise
+ */
+bool kvm_arm_verify_ext_dabt_pending(CPUState *cs);
+
 /**
  * its_class_name:
  *
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 3a46f54f1fd..8bb7318378b 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -749,6 +749,29 @@ int kvm_get_vcpu_events(ARMCPU *cpu)
 
 void kvm_arch_pre_run(CPUState *cs, struct kvm_run *run)
 {
+    ARMCPU *cpu = ARM_CPU(cs);
+    CPUARMState *env = &cpu->env;
+
+    if (unlikely(env->ext_dabt_raised)) {
+        /*
+         * Verifying that the ext DABT has been properly injected,
+         * otherwise risking indefinitely re-running the faulting instruction
+         * Covering a very narrow case for kernels 5.5..5.5.4
+         * when injected abort was misconfigured to be
+         * an IMPLEMENTATION DEFINED exception (for 32-bit EL1)
+         */
+        if (!arm_feature(env, ARM_FEATURE_AARCH64) &&
+            unlikely(!kvm_arm_verify_ext_dabt_pending(cs))) {
+
+            error_report("Data abort exception with no valid ISS generated by "
+                   "guest memory access. KVM unable to emulate faulting "
+                   "instruction. Failed to inject an external data abort "
+                   "into the guest.");
+            abort();
+       }
+       /* Clear the status */
+       env->ext_dabt_raised = 0;
+    }
 }
 
 MemTxAttrs kvm_arch_post_run(CPUState *cs, struct kvm_run *run)
@@ -833,6 +856,8 @@ void kvm_arm_vm_state_change(void *opaque, int running, RunState state)
 static int kvm_arm_handle_dabt_nisv(CPUState *cs, uint64_t esr_iss,
                                     uint64_t fault_ipa)
 {
+    ARMCPU *cpu = ARM_CPU(cs);
+    CPUARMState *env = &cpu->env;
     /*
      * Request KVM to inject the external data abort into the guest
      */
@@ -847,7 +872,10 @@ static int kvm_arm_handle_dabt_nisv(CPUState *cs, uint64_t esr_iss,
          */
         events.exception.ext_dabt_pending = 1;
         /* KVM_CAP_ARM_INJECT_EXT_DABT implies KVM_CAP_VCPU_EVENTS */
-        return kvm_vcpu_ioctl(cs, KVM_SET_VCPU_EVENTS, &events);
+        if (!kvm_vcpu_ioctl(cs, KVM_SET_VCPU_EVENTS, &events)) {
+            env->ext_dabt_raised = 1;
+            return 0;
+        }
     } else {
         error_report("Data abort exception triggered by guest memory access "
                      "at physical address: 0x"  TARGET_FMT_lx,
diff --git a/target/arm/kvm32.c b/target/arm/kvm32.c
index 7b3a19e9aef..0af46b41c84 100644
--- a/target/arm/kvm32.c
+++ b/target/arm/kvm32.c
@@ -559,3 +559,37 @@ void kvm_arm_pmu_init(CPUState *cs)
 {
     qemu_log_mask(LOG_UNIMP, "%s: not implemented\n", __func__);
 }
+
+#define ARM_REG_DFSR  ARM_CP15_REG32(0, 5, 0, 0)
+#define ARM_REG_TTBCR ARM_CP15_REG32(0, 2, 0, 2)
+/*
+ *DFSR:
+ *      TTBCR.EAE == 0
+ *          FS[4]   - DFSR[10]
+ *          FS[3:0] - DFSR[3:0]
+ *      TTBCR.EAE == 1
+ *          FS, bits [5:0]
+ */
+#define DFSR_FSC(lpae, v) \
+    ((lpae) ? ((v) & 0x3F) : (((v) >> 6) | ((v) & 0x1F)))
+
+#define DFSC_EXTABT(lpae) ((lpae) ? 0x10 : 0x08)
+
+bool kvm_arm_verify_ext_dabt_pending(CPUState *cs)
+{
+    uint32_t dfsr_val;
+
+    if (!kvm_get_one_reg(cs, ARM_REG_DFSR, &dfsr_val)) {
+        ARMCPU *cpu = ARM_CPU(cs);
+        CPUARMState *env = &cpu->env;
+        uint32_t ttbcr;
+        int lpae = 0;
+
+        if (!kvm_get_one_reg(cs, ARM_REG_TTBCR, &ttbcr)) {
+            lpae = arm_feature(env, ARM_FEATURE_LPAE) && (ttbcr & TTBCR_EAE);
+        }
+        /* The verification is based on FS filed of the DFSR reg only*/
+        return (DFSR_FSC(lpae, dfsr_val) == DFSC_EXTABT(lpae));
+    }
+    return false;
+}
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 3dc494aaa7e..11692379055 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -1493,3 +1493,52 @@ bool kvm_arm_handle_debug(CPUState *cs, struct kvm_debug_exit_arch *debug_exit)
 
     return false;
 }
+
+#define ARM64_REG_ESR_EL1 ARM64_SYS_REG(3, 0, 5, 2, 0)
+#define ARM64_REG_TCR_EL1 ARM64_SYS_REG(3, 0, 2, 0, 2)
+
+/*
+ * ESR_EL1
+ * ISS encoding
+ * AARCH64: DFSC,   bits [5:0]
+ * AARCH32:
+ *      TTBCR.EAE == 0
+ *          FS[4]   - DFSR[10]
+ *          FS[3:0] - DFSR[3:0]
+ *      TTBCR.EAE == 1
+ *          FS, bits [5:0]
+ */
+#define ESR_DFSC(aarch64, lpae, v)        \
+    ((aarch64 || (lpae)) ? ((v) & 0x3F)   \
+               : (((v) >> 6) | ((v) & 0x1F)))
+
+#define ESR_DFSC_EXTABT(aarch64, lpae) \
+    ((aarch64) ? 0x10 : (lpae) ? 0x10 : 0x8)
+
+bool kvm_arm_verify_ext_dabt_pending(CPUState *cs)
+{
+    uint64_t dfsr_val;
+
+    if (!kvm_get_one_reg(cs, ARM64_REG_ESR_EL1, &dfsr_val)) {
+        ARMCPU *cpu = ARM_CPU(cs);
+        CPUARMState *env = &cpu->env;
+        int aarch64_mode = arm_feature(env, ARM_FEATURE_AARCH64);
+        int lpae = 0;
+
+        if (!aarch64_mode) {
+            uint64_t ttbcr;
+
+            if (!kvm_get_one_reg(cs, ARM64_REG_TCR_EL1, &ttbcr)) {
+                lpae = arm_feature(env, ARM_FEATURE_LPAE)
+                        && (ttbcr & TTBCR_EAE);
+            }
+        }
+        /*
+         * The verification here is based on the DFSC bits
+         * of the ESR_EL1 reg only
+         */
+         return (ESR_DFSC(aarch64_mode, lpae, dfsr_val) ==
+                ESR_DFSC_EXTABT(aarch64_mode, lpae));
+    }
+    return false;
+}
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 11/34] tests/acpi: remove stale allowed tables
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (9 preceding siblings ...)
  2020-07-03 16:53 ` [PULL 10/34] target/arm: kvm: Handle misconfigured dabt injection Peter Maydell
@ 2020-07-03 16:53 ` Peter Maydell
  2020-07-03 16:53 ` [PULL 12/34] tests/acpi: virt: allow DSDT acpi table changes Peter Maydell
                   ` (24 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:53 UTC (permalink / raw)
  To: qemu-devel

From: Andrew Jones <drjones@redhat.com>

Fixes: 93dd625f8bf7 ("tests/acpi: update expected data files")
Signed-off-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Message-id: 20200629140938.17566-2-drjones@redhat.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 tests/qtest/bios-tables-test-allowed-diff.h | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index 8992f1f12b7..dfb8523c8bf 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,19 +1 @@
 /* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/pc/DSDT",
-"tests/data/acpi/pc/DSDT.acpihmat",
-"tests/data/acpi/pc/DSDT.bridge",
-"tests/data/acpi/pc/DSDT.cphp",
-"tests/data/acpi/pc/DSDT.dimmpxm",
-"tests/data/acpi/pc/DSDT.ipmikcs",
-"tests/data/acpi/pc/DSDT.memhp",
-"tests/data/acpi/pc/DSDT.numamem",
-"tests/data/acpi/q35/DSDT",
-"tests/data/acpi/q35/DSDT.acpihmat",
-"tests/data/acpi/q35/DSDT.bridge",
-"tests/data/acpi/q35/DSDT.cphp",
-"tests/data/acpi/q35/DSDT.dimmpxm",
-"tests/data/acpi/q35/DSDT.ipmibt",
-"tests/data/acpi/q35/DSDT.memhp",
-"tests/data/acpi/q35/DSDT.mmio64",
-"tests/data/acpi/q35/DSDT.numamem",
-"tests/data/acpi/q35/DSDT.tis",
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 12/34] tests/acpi: virt: allow DSDT acpi table changes
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (10 preceding siblings ...)
  2020-07-03 16:53 ` [PULL 11/34] tests/acpi: remove stale allowed tables Peter Maydell
@ 2020-07-03 16:53 ` Peter Maydell
  2020-07-03 16:53 ` [PULL 13/34] hw/arm/virt-acpi-build: Only expose flash on older machine types Peter Maydell
                   ` (23 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:53 UTC (permalink / raw)
  To: qemu-devel

From: Andrew Jones <drjones@redhat.com>

Signed-off-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Message-id: 20200629140938.17566-3-drjones@redhat.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 tests/qtest/bios-tables-test-allowed-diff.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8bf..32a401ae35f 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,4 @@
 /* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/virt/DSDT",
+"tests/data/acpi/virt/DSDT.memhp",
+"tests/data/acpi/virt/DSDT.numamem",
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 13/34] hw/arm/virt-acpi-build: Only expose flash on older machine types
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (11 preceding siblings ...)
  2020-07-03 16:53 ` [PULL 12/34] tests/acpi: virt: allow DSDT acpi table changes Peter Maydell
@ 2020-07-03 16:53 ` Peter Maydell
  2020-07-03 16:53 ` [PULL 14/34] tests/acpi: virt: update golden masters for DSDT Peter Maydell
                   ` (22 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:53 UTC (permalink / raw)
  To: qemu-devel

From: Andrew Jones <drjones@redhat.com>

The flash device is exclusively for the host-controlled firmware, so
we should not expose it to the OS. Exposing it risks the OS messing
with it, which could break firmware runtime services and surprise the
OS when all its changes disappear after reboot.

As firmware needs the device and uses DT, we leave the device exposed
there. It's up to firmware to remove the nodes from DT before sending
it on to the OS. However, there's no need to force firmware to remove
tables from ACPI (which it doesn't know how to do anyway), so we
simply don't add the tables in the first place. But, as we've been
adding the tables for quite some time and don't want to change the
default hardware exposed to versioned machines, then we only stop
exposing the flash device tables for 5.1 and later machine types.

Suggested-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
Suggested-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Message-id: 20200629140938.17566-4-drjones@redhat.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/virt.h    | 1 +
 hw/arm/virt-acpi-build.c | 5 ++++-
 hw/arm/virt.c            | 3 +++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index a18b6b397b6..54bcf17afd3 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -125,6 +125,7 @@ typedef struct {
     bool no_highmem_ecam;
     bool no_ged;   /* Machines < 4.2 has no support for ACPI GED device */
     bool kvm_no_adjvtime;
+    bool acpi_expose_flash;
 } VirtMachineClass;
 
 typedef struct {
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 1384a2cf2ab..91f0df7b13a 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -749,6 +749,7 @@ static void build_fadt_rev5(GArray *table_data, BIOSLinker *linker,
 static void
 build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
 {
+    VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
     Aml *scope, *dsdt;
     MachineState *ms = MACHINE(vms);
     const MemMapEntry *memmap = vms->memmap;
@@ -767,7 +768,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
     acpi_dsdt_add_cpus(scope, vms->smp_cpus);
     acpi_dsdt_add_uart(scope, &memmap[VIRT_UART],
                        (irqmap[VIRT_UART] + ARM_SPI_BASE));
-    acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]);
+    if (vmc->acpi_expose_flash) {
+        acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]);
+    }
     acpi_dsdt_add_fw_cfg(scope, &memmap[VIRT_FW_CFG]);
     acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
                     (irqmap[VIRT_MMIO] + ARM_SPI_BASE), NUM_VIRTIO_TRANSPORTS);
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 7922f3c23a5..c78972fb797 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2510,9 +2510,12 @@ DEFINE_VIRT_MACHINE_AS_LATEST(5, 1)
 
 static void virt_machine_5_0_options(MachineClass *mc)
 {
+    VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
+
     virt_machine_5_1_options(mc);
     compat_props_add(mc->compat_props, hw_compat_5_0, hw_compat_5_0_len);
     mc->numa_mem_supported = true;
+    vmc->acpi_expose_flash = true;
 }
 DEFINE_VIRT_MACHINE(5, 0)
 
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 14/34] tests/acpi: virt: update golden masters for DSDT
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (12 preceding siblings ...)
  2020-07-03 16:53 ` [PULL 13/34] hw/arm/virt-acpi-build: Only expose flash on older machine types Peter Maydell
@ 2020-07-03 16:53 ` Peter Maydell
  2020-07-03 16:53 ` [PULL 15/34] target/arm: Fix temp double-free in sve ldr/str Peter Maydell
                   ` (21 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:53 UTC (permalink / raw)
  To: qemu-devel

From: Andrew Jones <drjones@redhat.com>

Differences between disassembled ASL files for DSDT:

@@ -5,13 +5,13 @@
  *
  * Disassembling to symbolic ASL+ operators
  *
- * Disassembly of a, Mon Jun 29 09:50:01 2020
+ * Disassembly of b, Mon Jun 29 09:50:03 2020
  *
  * Original Table Header:
  *     Signature        "DSDT"
- *     Length           0x000014BB (5307)
+ *     Length           0x00001455 (5205)
  *     Revision         0x02
- *     Checksum         0xD1
+ *     Checksum         0xE1
  *     OEM ID           "BOCHS "
  *     OEM Table ID     "BXPCDSDT"
  *     OEM Revision     0x00000001 (1)
@@ -45,32 +45,6 @@
             })
         }

-        Device (FLS0)
-        {
-            Name (_HID, "LNRO0015")  // _HID: Hardware ID
-            Name (_UID, Zero)  // _UID: Unique ID
-            Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
-            {
-                Memory32Fixed (ReadWrite,
-                    0x00000000,         // Address Base
-                    0x04000000,         // Address Length
-                    )
-            })
-        }
-
-        Device (FLS1)
-        {
-            Name (_HID, "LNRO0015")  // _HID: Hardware ID
-            Name (_UID, One)  // _UID: Unique ID
-            Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
-            {
-                Memory32Fixed (ReadWrite,
-                    0x04000000,         // Address Base
-                    0x04000000,         // Address Length
-                    )
-            })
-        }
-
         Device (FWCF)
         {
             Name (_HID, "QEMU0002")  // _HID: Hardware ID

The other two binaries have the same changes (the removal of the
flash devices).

Signed-off-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Message-id: 20200629140938.17566-5-drjones@redhat.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 tests/qtest/bios-tables-test-allowed-diff.h |   3 ---
 tests/data/acpi/virt/DSDT                   | Bin 5307 -> 5205 bytes
 tests/data/acpi/virt/DSDT.memhp             | Bin 6668 -> 6566 bytes
 tests/data/acpi/virt/DSDT.numamem           | Bin 5307 -> 5205 bytes
 4 files changed, 3 deletions(-)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index 32a401ae35f..dfb8523c8bf 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,4 +1 @@
 /* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/virt/DSDT",
-"tests/data/acpi/virt/DSDT.memhp",
-"tests/data/acpi/virt/DSDT.numamem",
diff --git a/tests/data/acpi/virt/DSDT b/tests/data/acpi/virt/DSDT
index d6f5c617881c4247f55d4dcd06581f9693916b2f..e669508d175f1e3ddf355f8a9b0d419266cac8aa 100644
GIT binary patch
delta 28
kcmdn3c~yhUCD<h-RD^+n>ET2!X{H9}iRuX(-<}f&0DgxFc>n+a

delta 156
zcmcbrv0IbNCD<iow+I6R)5VEg(oAih6V(&y4c&Z#4LIUGJY9Hw{DS-q3=B;fIO0P+
zU4W!>P_UpN7hfAE10w?juv9WcH-WSmV$;Hiu7w4t3#`S$E!^1+q9xGPH`KtuzzAr5
LaERl^1zUvy_;n(J

diff --git a/tests/data/acpi/virt/DSDT.memhp b/tests/data/acpi/virt/DSDT.memhp
index 730e95a46d2cce0af011ffc051d7342beb8f1328..4cb81f692d73526542493a0c4da9c9793cc8366e 100644
GIT binary patch
delta 28
kcmeA%S!T@T66_MPOp<|tiD@F2G*jb@iRuX(-^xn@0CHUjRR910

delta 156
zcmZ2x++)J!66_MfBgMeL^l>7WG*kP$iRuaUhHgH=1|0Doo-VvTenI{Q28N~#9Py!^
zE<n;bC|FRCi?5B7fsp|MSSlH!n?PC&v1wsM*TMqS1=eEW7Vhi@(GuwD8){%+U<5Qj
LIK*+|0yaqism~!^

diff --git a/tests/data/acpi/virt/DSDT.numamem b/tests/data/acpi/virt/DSDT.numamem
index d6f5c617881c4247f55d4dcd06581f9693916b2f..e669508d175f1e3ddf355f8a9b0d419266cac8aa 100644
GIT binary patch
delta 28
kcmdn3c~yhUCD<h-RD^+n>ET2!X{H9}iRuX(-<}f&0DgxFc>n+a

delta 156
zcmcbrv0IbNCD<iow+I6R)5VEg(oAih6V(&y4c&Z#4LIUGJY9Hw{DS-q3=B;fIO0P+
zU4W!>P_UpN7hfAE10w?juv9WcH-WSmV$;Hiu7w4t3#`S$E!^1+q9xGPH`KtuzzAr5
LaERl^1zUvy_;n(J

-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 15/34] target/arm: Fix temp double-free in sve ldr/str
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (13 preceding siblings ...)
  2020-07-03 16:53 ` [PULL 14/34] tests/acpi: virt: update golden masters for DSDT Peter Maydell
@ 2020-07-03 16:53 ` Peter Maydell
  2020-07-03 16:53 ` [PULL 16/34] hw/display/bcm2835_fb.c: Initialize all fields of struct Peter Maydell
                   ` (20 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:53 UTC (permalink / raw)
  To: qemu-devel

From: Richard Henderson <richard.henderson@linaro.org>

The temp that gets assigned to clean_addr has been allocated with
new_tmp_a64, which means that it will be freed at the end of the
instruction.  Freeing it earlier leads to assertion failure.

The loop creates a complication, in which we allocate a new local
temp, which does need freeing, and the final code path is shared
between the loop and non-loop.

Fix this complication by adding new_tmp_a64_local so that the new
local temp is freed at the end, and can be treated exactly like
the non-loop path.

Fixes: bba87d0a0f4
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20200702175605.1987125-1-richard.henderson@linaro.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/translate-a64.h | 1 +
 target/arm/translate-a64.c | 6 ++++++
 target/arm/translate-sve.c | 8 ++------
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/target/arm/translate-a64.h b/target/arm/translate-a64.h
index 49e4865918d..647f0c74f62 100644
--- a/target/arm/translate-a64.h
+++ b/target/arm/translate-a64.h
@@ -30,6 +30,7 @@ void unallocated_encoding(DisasContext *s);
     } while (0)
 
 TCGv_i64 new_tmp_a64(DisasContext *s);
+TCGv_i64 new_tmp_a64_local(DisasContext *s);
 TCGv_i64 new_tmp_a64_zero(DisasContext *s);
 TCGv_i64 cpu_reg(DisasContext *s, int reg);
 TCGv_i64 cpu_reg_sp(DisasContext *s, int reg);
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 73d753f11fb..8c0764957c8 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -461,6 +461,12 @@ TCGv_i64 new_tmp_a64(DisasContext *s)
     return s->tmp_a64[s->tmp_a64_count++] = tcg_temp_new_i64();
 }
 
+TCGv_i64 new_tmp_a64_local(DisasContext *s)
+{
+    assert(s->tmp_a64_count < TMP_A64_MAX);
+    return s->tmp_a64[s->tmp_a64_count++] = tcg_temp_local_new_i64();
+}
+
 TCGv_i64 new_tmp_a64_zero(DisasContext *s)
 {
     TCGv_i64 t = new_tmp_a64(s);
diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c
index f318ca265f2..08f0fd15b28 100644
--- a/target/arm/translate-sve.c
+++ b/target/arm/translate-sve.c
@@ -4372,9 +4372,8 @@ static void do_ldr(DisasContext *s, uint32_t vofs, int len, int rn, int imm)
 
         /* Copy the clean address into a local temp, live across the loop. */
         t0 = clean_addr;
-        clean_addr = tcg_temp_local_new_i64();
+        clean_addr = new_tmp_a64_local(s);
         tcg_gen_mov_i64(clean_addr, t0);
-        tcg_temp_free_i64(t0);
 
         gen_set_label(loop);
 
@@ -4422,7 +4421,6 @@ static void do_ldr(DisasContext *s, uint32_t vofs, int len, int rn, int imm)
         tcg_gen_st_i64(t0, cpu_env, vofs + len_align);
         tcg_temp_free_i64(t0);
     }
-    tcg_temp_free_i64(clean_addr);
 }
 
 /* Similarly for stores.  */
@@ -4463,9 +4461,8 @@ static void do_str(DisasContext *s, uint32_t vofs, int len, int rn, int imm)
 
         /* Copy the clean address into a local temp, live across the loop. */
         t0 = clean_addr;
-        clean_addr = tcg_temp_local_new_i64();
+        clean_addr = new_tmp_a64_local(s);
         tcg_gen_mov_i64(clean_addr, t0);
-        tcg_temp_free_i64(t0);
 
         gen_set_label(loop);
 
@@ -4509,7 +4506,6 @@ static void do_str(DisasContext *s, uint32_t vofs, int len, int rn, int imm)
         }
         tcg_temp_free_i64(t0);
     }
-    tcg_temp_free_i64(clean_addr);
 }
 
 static bool trans_LDR_zri(DisasContext *s, arg_rri *a)
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 16/34] hw/display/bcm2835_fb.c: Initialize all fields of struct
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (14 preceding siblings ...)
  2020-07-03 16:53 ` [PULL 15/34] target/arm: Fix temp double-free in sve ldr/str Peter Maydell
@ 2020-07-03 16:53 ` Peter Maydell
  2020-07-03 16:53 ` [PULL 17/34] hw/arm/spitz: Detabify Peter Maydell
                   ` (19 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:53 UTC (permalink / raw)
  To: qemu-devel

In bcm2835_fb_mbox_push(), Coverity complains (CID 1429989) that we
pass a pointer to a local struct to another function without
initializing all its fields.  This is a real bug:
bcm2835_fb_reconfigure() copies the whole of our new BCM2385FBConfig
struct into s->config, so any fields we don't initialize will corrupt
the state of the device.

Copy the two fields which we don't want to update (pixo and alpha)
from the existing config so we don't accidentally change them.

Fixes: cfb7ba983857e40e88
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20200628195436.27582-1-peter.maydell@linaro.org
---
 hw/display/bcm2835_fb.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/display/bcm2835_fb.c b/hw/display/bcm2835_fb.c
index c6263808a27..7c0e5eef2d5 100644
--- a/hw/display/bcm2835_fb.c
+++ b/hw/display/bcm2835_fb.c
@@ -282,6 +282,10 @@ static void bcm2835_fb_mbox_push(BCM2835FBState *s, uint32_t value)
     newconf.base = s->vcram_base | (value & 0xc0000000);
     newconf.base += BCM2835_FB_OFFSET;
 
+    /* Copy fields which we don't want to change from the existing config */
+    newconf.pixo = s->config.pixo;
+    newconf.alpha = s->config.alpha;
+
     bcm2835_fb_validate_config(&newconf);
 
     pitch = bcm2835_fb_get_pitch(&newconf);
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 17/34] hw/arm/spitz: Detabify
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (15 preceding siblings ...)
  2020-07-03 16:53 ` [PULL 16/34] hw/display/bcm2835_fb.c: Initialize all fields of struct Peter Maydell
@ 2020-07-03 16:53 ` Peter Maydell
  2020-07-03 16:53 ` [PULL 18/34] hw/arm/spitz: Create SpitzMachineClass abstract base class Peter Maydell
                   ` (18 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:53 UTC (permalink / raw)
  To: qemu-devel

The spitz board has been around a long time, and still has a fair number
of hard-coded tab characters in it. We're about to do some work on
this source file, so start out by expanding out the tabs.

This commit is a pure whitespace only change.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-id: 20200628142429.17111-2-peter.maydell@linaro.org
---
 hw/arm/spitz.c | 156 ++++++++++++++++++++++++-------------------------
 1 file changed, 78 insertions(+), 78 deletions(-)

diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index fc18212e686..9eaedab79b5 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -34,25 +34,25 @@
 #include "cpu.h"
 
 #undef REG_FMT
-#define REG_FMT			"0x%02lx"
+#define REG_FMT                         "0x%02lx"
 
 /* Spitz Flash */
-#define FLASH_BASE		0x0c000000
-#define FLASH_ECCLPLB		0x00	/* Line parity 7 - 0 bit */
-#define FLASH_ECCLPUB		0x04	/* Line parity 15 - 8 bit */
-#define FLASH_ECCCP		0x08	/* Column parity 5 - 0 bit */
-#define FLASH_ECCCNTR		0x0c	/* ECC byte counter */
-#define FLASH_ECCCLRR		0x10	/* Clear ECC */
-#define FLASH_FLASHIO		0x14	/* Flash I/O */
-#define FLASH_FLASHCTL		0x18	/* Flash Control */
+#define FLASH_BASE              0x0c000000
+#define FLASH_ECCLPLB           0x00    /* Line parity 7 - 0 bit */
+#define FLASH_ECCLPUB           0x04    /* Line parity 15 - 8 bit */
+#define FLASH_ECCCP             0x08    /* Column parity 5 - 0 bit */
+#define FLASH_ECCCNTR           0x0c    /* ECC byte counter */
+#define FLASH_ECCCLRR           0x10    /* Clear ECC */
+#define FLASH_FLASHIO           0x14    /* Flash I/O */
+#define FLASH_FLASHCTL          0x18    /* Flash Control */
 
-#define FLASHCTL_CE0		(1 << 0)
-#define FLASHCTL_CLE		(1 << 1)
-#define FLASHCTL_ALE		(1 << 2)
-#define FLASHCTL_WP		(1 << 3)
-#define FLASHCTL_CE1		(1 << 4)
-#define FLASHCTL_RYBY		(1 << 5)
-#define FLASHCTL_NCE		(FLASHCTL_CE0 | FLASHCTL_CE1)
+#define FLASHCTL_CE0            (1 << 0)
+#define FLASHCTL_CLE            (1 << 1)
+#define FLASHCTL_ALE            (1 << 2)
+#define FLASHCTL_WP             (1 << 3)
+#define FLASHCTL_CE1            (1 << 4)
+#define FLASHCTL_RYBY           (1 << 5)
+#define FLASHCTL_NCE            (FLASHCTL_CE0 | FLASHCTL_CE1)
 
 #define TYPE_SL_NAND "sl-nand"
 #define SL_NAND(obj) OBJECT_CHECK(SLNANDState, (obj), TYPE_SL_NAND)
@@ -74,12 +74,12 @@ static uint64_t sl_read(void *opaque, hwaddr addr, unsigned size)
     int ryby;
 
     switch (addr) {
-#define BSHR(byte, from, to)	((s->ecc.lp[byte] >> (from - to)) & (1 << to))
+#define BSHR(byte, from, to)    ((s->ecc.lp[byte] >> (from - to)) & (1 << to))
     case FLASH_ECCLPLB:
         return BSHR(0, 4, 0) | BSHR(0, 5, 2) | BSHR(0, 6, 4) | BSHR(0, 7, 6) |
                 BSHR(1, 4, 1) | BSHR(1, 5, 3) | BSHR(1, 6, 5) | BSHR(1, 7, 7);
 
-#define BSHL(byte, from, to)	((s->ecc.lp[byte] << (to - from)) & (1 << to))
+#define BSHL(byte, from, to)    ((s->ecc.lp[byte] << (to - from)) & (1 << to))
     case FLASH_ECCLPUB:
         return BSHL(0, 0, 0) | BSHL(0, 1, 2) | BSHL(0, 2, 4) | BSHL(0, 3, 6) |
                 BSHL(1, 0, 1) | BSHL(1, 1, 3) | BSHL(1, 2, 5) | BSHL(1, 3, 7);
@@ -191,8 +191,8 @@ static void sl_nand_realize(DeviceState *dev, Error **errp)
 
 /* Spitz Keyboard */
 
-#define SPITZ_KEY_STROBE_NUM	11
-#define SPITZ_KEY_SENSE_NUM	7
+#define SPITZ_KEY_STROBE_NUM    11
+#define SPITZ_KEY_SENSE_NUM     7
 
 static const int spitz_gpio_key_sense[SPITZ_KEY_SENSE_NUM] = {
     12, 17, 91, 34, 36, 38, 39
@@ -214,11 +214,11 @@ static int spitz_keymap[SPITZ_KEY_SENSE_NUM + 1][SPITZ_KEY_STROBE_NUM] = {
     { 0x52, 0x43, 0x01, 0x47, 0x49,  -1 ,  -1 ,  -1 ,  -1 ,  -1 ,  -1  },
 };
 
-#define SPITZ_GPIO_AK_INT	13	/* Remote control */
-#define SPITZ_GPIO_SYNC		16	/* Sync button */
-#define SPITZ_GPIO_ON_KEY	95	/* Power button */
-#define SPITZ_GPIO_SWA		97	/* Lid */
-#define SPITZ_GPIO_SWB		96	/* Tablet mode */
+#define SPITZ_GPIO_AK_INT       13      /* Remote control */
+#define SPITZ_GPIO_SYNC                 16      /* Sync button */
+#define SPITZ_GPIO_ON_KEY       95      /* Power button */
+#define SPITZ_GPIO_SWA          97      /* Lid */
+#define SPITZ_GPIO_SWB          96      /* Tablet mode */
 
 /* The special buttons are mapped to unused keys */
 static const int spitz_gpiomap[5] = {
@@ -300,7 +300,7 @@ static void spitz_keyboard_keydown(SpitzKeyboardState *s, int keycode)
 #define SPITZ_MOD_CTRL    (1 << 8)
 #define SPITZ_MOD_FN      (1 << 9)
 
-#define QUEUE_KEY(c)	s->fifo[(s->fifopos + s->fifolen ++) & 0xf] = c
+#define QUEUE_KEY(c)    s->fifo[(s->fifopos + s->fifolen ++) & 0xf] = c
 
 static void spitz_keyboard_handler(void *opaque, int keycode)
 {
@@ -308,25 +308,25 @@ static void spitz_keyboard_handler(void *opaque, int keycode)
     uint16_t code;
     int mapcode;
     switch (keycode) {
-    case 0x2a:	/* Left Shift */
+    case 0x2a:  /* Left Shift */
         s->modifiers |= 1;
         break;
     case 0xaa:
         s->modifiers &= ~1;
         break;
-    case 0x36:	/* Right Shift */
+    case 0x36:  /* Right Shift */
         s->modifiers |= 2;
         break;
     case 0xb6:
         s->modifiers &= ~2;
         break;
-    case 0x1d:	/* Control */
+    case 0x1d:  /* Control */
         s->modifiers |= 4;
         break;
     case 0x9d:
         s->modifiers &= ~4;
         break;
-    case 0x38:	/* Alt */
+    case 0x38:  /* Alt */
         s->modifiers |= 8;
         break;
     case 0xb8:
@@ -536,14 +536,14 @@ static void spitz_keyboard_realize(DeviceState *dev, Error **errp)
 
 /* LCD backlight controller */
 
-#define LCDTG_RESCTL	0x00
-#define LCDTG_PHACTRL	0x01
-#define LCDTG_DUTYCTRL	0x02
-#define LCDTG_POWERREG0	0x03
-#define LCDTG_POWERREG1	0x04
-#define LCDTG_GPOR3	0x05
-#define LCDTG_PICTRL	0x06
-#define LCDTG_POLCTRL	0x07
+#define LCDTG_RESCTL    0x00
+#define LCDTG_PHACTRL   0x01
+#define LCDTG_DUTYCTRL  0x02
+#define LCDTG_POWERREG0         0x03
+#define LCDTG_POWERREG1         0x04
+#define LCDTG_GPOR3     0x05
+#define LCDTG_PICTRL    0x06
+#define LCDTG_POLCTRL   0x07
 
 typedef struct {
     SSISlave ssidev;
@@ -623,12 +623,12 @@ static void spitz_lcdtg_realize(SSISlave *dev, Error **errp)
 
 /* SSP devices */
 
-#define CORGI_SSP_PORT		2
+#define CORGI_SSP_PORT          2
 
-#define SPITZ_GPIO_LCDCON_CS	53
-#define SPITZ_GPIO_ADS7846_CS	14
-#define SPITZ_GPIO_MAX1111_CS	20
-#define SPITZ_GPIO_TP_INT	11
+#define SPITZ_GPIO_LCDCON_CS    53
+#define SPITZ_GPIO_ADS7846_CS   14
+#define SPITZ_GPIO_MAX1111_CS   20
+#define SPITZ_GPIO_TP_INT       11
 
 static DeviceState *max1111;
 
@@ -659,13 +659,13 @@ static void corgi_ssp_gpio_cs(void *opaque, int line, int level)
     s->enable[line] = !level;
 }
 
-#define MAX1111_BATT_VOLT	1
-#define MAX1111_BATT_TEMP	2
-#define MAX1111_ACIN_VOLT	3
+#define MAX1111_BATT_VOLT       1
+#define MAX1111_BATT_TEMP       2
+#define MAX1111_ACIN_VOLT       3
 
-#define SPITZ_BATTERY_TEMP	0xe0	/* About 2.9V */
-#define SPITZ_BATTERY_VOLT	0xd0	/* About 4.0V */
-#define SPITZ_CHARGEON_ACIN	0x80	/* About 5.0V */
+#define SPITZ_BATTERY_TEMP      0xe0    /* About 2.9V */
+#define SPITZ_BATTERY_VOLT      0xd0    /* About 4.0V */
+#define SPITZ_CHARGEON_ACIN     0x80    /* About 5.0V */
 
 static void spitz_adc_temp_on(void *opaque, int line, int level)
 {
@@ -735,11 +735,11 @@ static void spitz_microdrive_attach(PXA2xxState *cpu, int slot)
 
 /* Wm8750 and Max7310 on I2C */
 
-#define AKITA_MAX_ADDR	0x18
-#define SPITZ_WM_ADDRL	0x1b
-#define SPITZ_WM_ADDRH	0x1a
+#define AKITA_MAX_ADDR  0x18
+#define SPITZ_WM_ADDRL  0x1b
+#define SPITZ_WM_ADDRH  0x1a
 
-#define SPITZ_GPIO_WM	5
+#define SPITZ_GPIO_WM   5
 
 static void spitz_wm8750_addr(void *opaque, int line, int level)
 {
@@ -806,20 +806,20 @@ static void spitz_out_switch(void *opaque, int line, int level)
     }
 }
 
-#define SPITZ_SCP_LED_GREEN		1
-#define SPITZ_SCP_JK_B			2
-#define SPITZ_SCP_CHRG_ON		3
-#define SPITZ_SCP_MUTE_L		4
-#define SPITZ_SCP_MUTE_R		5
-#define SPITZ_SCP_CF_POWER		6
-#define SPITZ_SCP_LED_ORANGE		7
-#define SPITZ_SCP_JK_A			8
-#define SPITZ_SCP_ADC_TEMP_ON		9
-#define SPITZ_SCP2_IR_ON		1
-#define SPITZ_SCP2_AKIN_PULLUP		2
-#define SPITZ_SCP2_BACKLIGHT_CONT	7
-#define SPITZ_SCP2_BACKLIGHT_ON		8
-#define SPITZ_SCP2_MIC_BIAS		9
+#define SPITZ_SCP_LED_GREEN             1
+#define SPITZ_SCP_JK_B                  2
+#define SPITZ_SCP_CHRG_ON               3
+#define SPITZ_SCP_MUTE_L                4
+#define SPITZ_SCP_MUTE_R                5
+#define SPITZ_SCP_CF_POWER              6
+#define SPITZ_SCP_LED_ORANGE            7
+#define SPITZ_SCP_JK_A                  8
+#define SPITZ_SCP_ADC_TEMP_ON           9
+#define SPITZ_SCP2_IR_ON                1
+#define SPITZ_SCP2_AKIN_PULLUP          2
+#define SPITZ_SCP2_BACKLIGHT_CONT       7
+#define SPITZ_SCP2_BACKLIGHT_ON                 8
+#define SPITZ_SCP2_MIC_BIAS             9
 
 static void spitz_scoop_gpio_setup(PXA2xxState *cpu,
                 DeviceState *scp0, DeviceState *scp1)
@@ -839,15 +839,15 @@ static void spitz_scoop_gpio_setup(PXA2xxState *cpu,
     qdev_connect_gpio_out(scp0, SPITZ_SCP_ADC_TEMP_ON, outsignals[6]);
 }
 
-#define SPITZ_GPIO_HSYNC		22
-#define SPITZ_GPIO_SD_DETECT		9
-#define SPITZ_GPIO_SD_WP		81
-#define SPITZ_GPIO_ON_RESET		89
-#define SPITZ_GPIO_BAT_COVER		90
-#define SPITZ_GPIO_CF1_IRQ		105
-#define SPITZ_GPIO_CF1_CD		94
-#define SPITZ_GPIO_CF2_IRQ		106
-#define SPITZ_GPIO_CF2_CD		93
+#define SPITZ_GPIO_HSYNC                22
+#define SPITZ_GPIO_SD_DETECT            9
+#define SPITZ_GPIO_SD_WP                81
+#define SPITZ_GPIO_ON_RESET             89
+#define SPITZ_GPIO_BAT_COVER            90
+#define SPITZ_GPIO_CF1_IRQ              105
+#define SPITZ_GPIO_CF1_CD               94
+#define SPITZ_GPIO_CF2_IRQ              106
+#define SPITZ_GPIO_CF2_CD               93
 
 static int spitz_hsync;
 
@@ -907,8 +907,8 @@ static void spitz_gpio_setup(PXA2xxState *cpu, int slots)
 /* Board init.  */
 enum spitz_model_e { spitz, akita, borzoi, terrier };
 
-#define SPITZ_RAM	0x04000000
-#define SPITZ_ROM	0x00800000
+#define SPITZ_RAM       0x04000000
+#define SPITZ_ROM       0x00800000
 
 static struct arm_boot_info spitz_binfo = {
     .loader_start = PXA2XX_SDRAM_BASE,
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 18/34] hw/arm/spitz: Create SpitzMachineClass abstract base class
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (16 preceding siblings ...)
  2020-07-03 16:53 ` [PULL 17/34] hw/arm/spitz: Detabify Peter Maydell
@ 2020-07-03 16:53 ` Peter Maydell
  2020-07-03 16:53 ` [PULL 19/34] hw/arm/spitz: Keep pointers to MPU and SSI devices in SpitzMachineState Peter Maydell
                   ` (17 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:53 UTC (permalink / raw)
  To: qemu-devel

For the four Spitz-family machines (akita, borzoi, spitz, terrier)
create a proper abstract class SpitzMachineClass which encapsulates
the common behaviour, rather than having them all derive directly
from TYPE_MACHINE:
 * instead of each machine class setting mc->init to a wrapper
   function which calls spitz_common_init() with parameters,
   put that data in the SpitzMachineClass and make spitz_common_init
   the SpitzMachineClass machine-init function
 * move the settings of mc->block_default_type and
   mc->ignore_memory_transaction_failures into the SpitzMachineClass
   class init rather than repeating them in each machine's class init

(The motivation is that we're going to want to keep some state in
the SpitzMachineState so we can connect GPIOs between devices created
in one sub-function of the machine init to devices created in a
different sub-function.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20200628142429.17111-3-peter.maydell@linaro.org
---
 hw/arm/spitz.c | 91 ++++++++++++++++++++++++++++++--------------------
 1 file changed, 55 insertions(+), 36 deletions(-)

diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index 9eaedab79b5..c70e912a33d 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -33,6 +33,26 @@
 #include "exec/address-spaces.h"
 #include "cpu.h"
 
+enum spitz_model_e { spitz, akita, borzoi, terrier };
+
+typedef struct {
+    MachineClass parent;
+    enum spitz_model_e model;
+    int arm_id;
+} SpitzMachineClass;
+
+typedef struct {
+    MachineState parent;
+} SpitzMachineState;
+
+#define TYPE_SPITZ_MACHINE "spitz-common"
+#define SPITZ_MACHINE(obj) \
+    OBJECT_CHECK(SpitzMachineState, obj, TYPE_SPITZ_MACHINE)
+#define SPITZ_MACHINE_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(SpitzMachineClass, obj, TYPE_SPITZ_MACHINE)
+#define SPITZ_MACHINE_CLASS(klass) \
+    OBJECT_CLASS_CHECK(SpitzMachineClass, klass, TYPE_SPITZ_MACHINE)
+
 #undef REG_FMT
 #define REG_FMT                         "0x%02lx"
 
@@ -905,8 +925,6 @@ static void spitz_gpio_setup(PXA2xxState *cpu, int slots)
 }
 
 /* Board init.  */
-enum spitz_model_e { spitz, akita, borzoi, terrier };
-
 #define SPITZ_RAM       0x04000000
 #define SPITZ_ROM       0x00800000
 
@@ -915,9 +933,10 @@ static struct arm_boot_info spitz_binfo = {
     .ram_size = 0x04000000,
 };
 
-static void spitz_common_init(MachineState *machine,
-                              enum spitz_model_e model, int arm_id)
+static void spitz_common_init(MachineState *machine)
 {
+    SpitzMachineClass *smc = SPITZ_MACHINE_GET_CLASS(machine);
+    enum spitz_model_e model = smc->model;
     PXA2xxState *mpu;
     DeviceState *scp0, *scp1 = NULL;
     MemoryRegion *address_space_mem = get_system_memory();
@@ -958,100 +977,100 @@ static void spitz_common_init(MachineState *machine,
         /* A 4.0 GB microdrive is permanently sitting in CF slot 0.  */
         spitz_microdrive_attach(mpu, 0);
 
-    spitz_binfo.board_id = arm_id;
+    spitz_binfo.board_id = smc->arm_id;
     arm_load_kernel(mpu->cpu, machine, &spitz_binfo);
     sl_bootparam_write(SL_PXA_PARAM_BASE);
 }
 
-static void spitz_init(MachineState *machine)
+static void spitz_common_class_init(ObjectClass *oc, void *data)
 {
-    spitz_common_init(machine, spitz, 0x2c9);
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    mc->block_default_type = IF_IDE;
+    mc->ignore_memory_transaction_failures = true;
+    mc->init = spitz_common_init;
 }
 
-static void borzoi_init(MachineState *machine)
-{
-    spitz_common_init(machine, borzoi, 0x33f);
-}
-
-static void akita_init(MachineState *machine)
-{
-    spitz_common_init(machine, akita, 0x2e8);
-}
-
-static void terrier_init(MachineState *machine)
-{
-    spitz_common_init(machine, terrier, 0x33f);
-}
+static const TypeInfo spitz_common_info = {
+    .name = TYPE_SPITZ_MACHINE,
+    .parent = TYPE_MACHINE,
+    .abstract = true,
+    .instance_size = sizeof(SpitzMachineState),
+    .class_size = sizeof(SpitzMachineClass),
+    .class_init = spitz_common_class_init,
+};
 
 static void akitapda_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
+    SpitzMachineClass *smc = SPITZ_MACHINE_CLASS(oc);
 
     mc->desc = "Sharp SL-C1000 (Akita) PDA (PXA270)";
-    mc->init = akita_init;
-    mc->ignore_memory_transaction_failures = true;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("pxa270-c0");
+    smc->model = akita;
+    smc->arm_id = 0x2e8;
 }
 
 static const TypeInfo akitapda_type = {
     .name = MACHINE_TYPE_NAME("akita"),
-    .parent = TYPE_MACHINE,
+    .parent = TYPE_SPITZ_MACHINE,
     .class_init = akitapda_class_init,
 };
 
 static void spitzpda_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
+    SpitzMachineClass *smc = SPITZ_MACHINE_CLASS(oc);
 
     mc->desc = "Sharp SL-C3000 (Spitz) PDA (PXA270)";
-    mc->init = spitz_init;
-    mc->block_default_type = IF_IDE;
-    mc->ignore_memory_transaction_failures = true;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("pxa270-c0");
+    smc->model = spitz;
+    smc->arm_id = 0x2c9;
 }
 
 static const TypeInfo spitzpda_type = {
     .name = MACHINE_TYPE_NAME("spitz"),
-    .parent = TYPE_MACHINE,
+    .parent = TYPE_SPITZ_MACHINE,
     .class_init = spitzpda_class_init,
 };
 
 static void borzoipda_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
+    SpitzMachineClass *smc = SPITZ_MACHINE_CLASS(oc);
 
     mc->desc = "Sharp SL-C3100 (Borzoi) PDA (PXA270)";
-    mc->init = borzoi_init;
-    mc->block_default_type = IF_IDE;
-    mc->ignore_memory_transaction_failures = true;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("pxa270-c0");
+    smc->model = borzoi;
+    smc->arm_id = 0x33f;
 }
 
 static const TypeInfo borzoipda_type = {
     .name = MACHINE_TYPE_NAME("borzoi"),
-    .parent = TYPE_MACHINE,
+    .parent = TYPE_SPITZ_MACHINE,
     .class_init = borzoipda_class_init,
 };
 
 static void terrierpda_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
+    SpitzMachineClass *smc = SPITZ_MACHINE_CLASS(oc);
 
     mc->desc = "Sharp SL-C3200 (Terrier) PDA (PXA270)";
-    mc->init = terrier_init;
-    mc->block_default_type = IF_IDE;
-    mc->ignore_memory_transaction_failures = true;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("pxa270-c5");
+    smc->model = terrier;
+    smc->arm_id = 0x33f;
 }
 
 static const TypeInfo terrierpda_type = {
     .name = MACHINE_TYPE_NAME("terrier"),
-    .parent = TYPE_MACHINE,
+    .parent = TYPE_SPITZ_MACHINE,
     .class_init = terrierpda_class_init,
 };
 
 static void spitz_machine_init(void)
 {
+    type_register_static(&spitz_common_info);
     type_register_static(&akitapda_type);
     type_register_static(&spitzpda_type);
     type_register_static(&borzoipda_type);
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 19/34] hw/arm/spitz: Keep pointers to MPU and SSI devices in SpitzMachineState
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (17 preceding siblings ...)
  2020-07-03 16:53 ` [PULL 18/34] hw/arm/spitz: Create SpitzMachineClass abstract base class Peter Maydell
@ 2020-07-03 16:53 ` Peter Maydell
  2020-07-03 16:53 ` [PULL 20/34] hw/arm/spitz: Keep pointers to scp0, scp1 " Peter Maydell
                   ` (16 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:53 UTC (permalink / raw)
  To: qemu-devel

Keep pointers to the MPU and the SSI devices in SpitzMachineState.
We're going to want to make GPIO connections between some of the
SSI devices and the SCPs, so we want to keep hold of a pointer to
those; putting the MPU into the struct allows us to pass just
one thing to spitz_ssp_attach() rather than two.

We have to retain the setting of the global "max1111" variable
for the moment as it is used in spitz_adc_temp_on(); later in
this series of commits we will be able to remove it.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-id: 20200628142429.17111-4-peter.maydell@linaro.org
---
 hw/arm/spitz.c | 50 ++++++++++++++++++++++++++++----------------------
 1 file changed, 28 insertions(+), 22 deletions(-)

diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index c70e912a33d..f48e966c047 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -43,6 +43,11 @@ typedef struct {
 
 typedef struct {
     MachineState parent;
+    PXA2xxState *mpu;
+    DeviceState *mux;
+    DeviceState *lcdtg;
+    DeviceState *ads7846;
+    DeviceState *max1111;
 } SpitzMachineState;
 
 #define TYPE_SPITZ_MACHINE "spitz-common"
@@ -709,34 +714,33 @@ static void corgi_ssp_realize(SSISlave *d, Error **errp)
     s->bus[2] = ssi_create_bus(dev, "ssi2");
 }
 
-static void spitz_ssp_attach(PXA2xxState *cpu)
+static void spitz_ssp_attach(SpitzMachineState *sms)
 {
-    DeviceState *mux;
-    DeviceState *dev;
     void *bus;
 
-    mux = ssi_create_slave(cpu->ssp[CORGI_SSP_PORT - 1], "corgi-ssp");
+    sms->mux = ssi_create_slave(sms->mpu->ssp[CORGI_SSP_PORT - 1], "corgi-ssp");
 
-    bus = qdev_get_child_bus(mux, "ssi0");
-    ssi_create_slave(bus, "spitz-lcdtg");
+    bus = qdev_get_child_bus(sms->mux, "ssi0");
+    sms->lcdtg = ssi_create_slave(bus, "spitz-lcdtg");
 
-    bus = qdev_get_child_bus(mux, "ssi1");
-    dev = ssi_create_slave(bus, "ads7846");
-    qdev_connect_gpio_out(dev, 0,
-                          qdev_get_gpio_in(cpu->gpio, SPITZ_GPIO_TP_INT));
+    bus = qdev_get_child_bus(sms->mux, "ssi1");
+    sms->ads7846 = ssi_create_slave(bus, "ads7846");
+    qdev_connect_gpio_out(sms->ads7846, 0,
+                          qdev_get_gpio_in(sms->mpu->gpio, SPITZ_GPIO_TP_INT));
 
-    bus = qdev_get_child_bus(mux, "ssi2");
-    max1111 = ssi_create_slave(bus, "max1111");
-    max111x_set_input(max1111, MAX1111_BATT_VOLT, SPITZ_BATTERY_VOLT);
-    max111x_set_input(max1111, MAX1111_BATT_TEMP, 0);
-    max111x_set_input(max1111, MAX1111_ACIN_VOLT, SPITZ_CHARGEON_ACIN);
+    bus = qdev_get_child_bus(sms->mux, "ssi2");
+    sms->max1111 = ssi_create_slave(bus, "max1111");
+    max1111 = sms->max1111;
+    max111x_set_input(sms->max1111, MAX1111_BATT_VOLT, SPITZ_BATTERY_VOLT);
+    max111x_set_input(sms->max1111, MAX1111_BATT_TEMP, 0);
+    max111x_set_input(sms->max1111, MAX1111_ACIN_VOLT, SPITZ_CHARGEON_ACIN);
 
-    qdev_connect_gpio_out(cpu->gpio, SPITZ_GPIO_LCDCON_CS,
-                        qdev_get_gpio_in(mux, 0));
-    qdev_connect_gpio_out(cpu->gpio, SPITZ_GPIO_ADS7846_CS,
-                        qdev_get_gpio_in(mux, 1));
-    qdev_connect_gpio_out(cpu->gpio, SPITZ_GPIO_MAX1111_CS,
-                        qdev_get_gpio_in(mux, 2));
+    qdev_connect_gpio_out(sms->mpu->gpio, SPITZ_GPIO_LCDCON_CS,
+                        qdev_get_gpio_in(sms->mux, 0));
+    qdev_connect_gpio_out(sms->mpu->gpio, SPITZ_GPIO_ADS7846_CS,
+                        qdev_get_gpio_in(sms->mux, 1));
+    qdev_connect_gpio_out(sms->mpu->gpio, SPITZ_GPIO_MAX1111_CS,
+                        qdev_get_gpio_in(sms->mux, 2));
 }
 
 /* CF Microdrive */
@@ -936,6 +940,7 @@ static struct arm_boot_info spitz_binfo = {
 static void spitz_common_init(MachineState *machine)
 {
     SpitzMachineClass *smc = SPITZ_MACHINE_GET_CLASS(machine);
+    SpitzMachineState *sms = SPITZ_MACHINE(machine);
     enum spitz_model_e model = smc->model;
     PXA2xxState *mpu;
     DeviceState *scp0, *scp1 = NULL;
@@ -945,6 +950,7 @@ static void spitz_common_init(MachineState *machine)
     /* Setup CPU & memory */
     mpu = pxa270_init(address_space_mem, spitz_binfo.ram_size,
                       machine->cpu_type);
+    sms->mpu = mpu;
 
     sl_flash_register(mpu, (model == spitz) ? FLASH_128M : FLASH_1024M);
 
@@ -954,7 +960,7 @@ static void spitz_common_init(MachineState *machine)
     /* Setup peripherals */
     spitz_keyboard_register(mpu);
 
-    spitz_ssp_attach(mpu);
+    spitz_ssp_attach(sms);
 
     scp0 = sysbus_create_simple("scoop", 0x10800000, NULL);
     if (model != akita) {
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 20/34] hw/arm/spitz: Keep pointers to scp0, scp1 in SpitzMachineState
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (18 preceding siblings ...)
  2020-07-03 16:53 ` [PULL 19/34] hw/arm/spitz: Keep pointers to MPU and SSI devices in SpitzMachineState Peter Maydell
@ 2020-07-03 16:53 ` Peter Maydell
  2020-07-03 16:53 ` [PULL 21/34] hw/arm/spitz: Implement inbound GPIO lines for bit5 and power signals Peter Maydell
                   ` (15 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:53 UTC (permalink / raw)
  To: qemu-devel

Keep pointers to scp0, scp1 in SpitzMachineState, and just pass
that to spitz_scoop_gpio_setup().

(We'll want to use some of the other fields in SpitzMachineState
in that function in the next commit.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-id: 20200628142429.17111-5-peter.maydell@linaro.org
---
 hw/arm/spitz.c | 34 +++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index f48e966c047..69bc2b3fa10 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -48,6 +48,8 @@ typedef struct {
     DeviceState *lcdtg;
     DeviceState *ads7846;
     DeviceState *max1111;
+    DeviceState *scp0;
+    DeviceState *scp1;
 } SpitzMachineState;
 
 #define TYPE_SPITZ_MACHINE "spitz-common"
@@ -845,22 +847,23 @@ static void spitz_out_switch(void *opaque, int line, int level)
 #define SPITZ_SCP2_BACKLIGHT_ON                 8
 #define SPITZ_SCP2_MIC_BIAS             9
 
-static void spitz_scoop_gpio_setup(PXA2xxState *cpu,
-                DeviceState *scp0, DeviceState *scp1)
+static void spitz_scoop_gpio_setup(SpitzMachineState *sms)
 {
-    qemu_irq *outsignals = qemu_allocate_irqs(spitz_out_switch, cpu, 8);
+    qemu_irq *outsignals = qemu_allocate_irqs(spitz_out_switch, sms->mpu, 8);
 
-    qdev_connect_gpio_out(scp0, SPITZ_SCP_CHRG_ON, outsignals[0]);
-    qdev_connect_gpio_out(scp0, SPITZ_SCP_JK_B, outsignals[1]);
-    qdev_connect_gpio_out(scp0, SPITZ_SCP_LED_GREEN, outsignals[2]);
-    qdev_connect_gpio_out(scp0, SPITZ_SCP_LED_ORANGE, outsignals[3]);
+    qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_CHRG_ON, outsignals[0]);
+    qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_JK_B, outsignals[1]);
+    qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_LED_GREEN, outsignals[2]);
+    qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_LED_ORANGE, outsignals[3]);
 
-    if (scp1) {
-        qdev_connect_gpio_out(scp1, SPITZ_SCP2_BACKLIGHT_CONT, outsignals[4]);
-        qdev_connect_gpio_out(scp1, SPITZ_SCP2_BACKLIGHT_ON, outsignals[5]);
+    if (sms->scp1) {
+        qdev_connect_gpio_out(sms->scp1, SPITZ_SCP2_BACKLIGHT_CONT,
+                              outsignals[4]);
+        qdev_connect_gpio_out(sms->scp1, SPITZ_SCP2_BACKLIGHT_ON,
+                              outsignals[5]);
     }
 
-    qdev_connect_gpio_out(scp0, SPITZ_SCP_ADC_TEMP_ON, outsignals[6]);
+    qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_ADC_TEMP_ON, outsignals[6]);
 }
 
 #define SPITZ_GPIO_HSYNC                22
@@ -943,7 +946,6 @@ static void spitz_common_init(MachineState *machine)
     SpitzMachineState *sms = SPITZ_MACHINE(machine);
     enum spitz_model_e model = smc->model;
     PXA2xxState *mpu;
-    DeviceState *scp0, *scp1 = NULL;
     MemoryRegion *address_space_mem = get_system_memory();
     MemoryRegion *rom = g_new(MemoryRegion, 1);
 
@@ -962,12 +964,14 @@ static void spitz_common_init(MachineState *machine)
 
     spitz_ssp_attach(sms);
 
-    scp0 = sysbus_create_simple("scoop", 0x10800000, NULL);
+    sms->scp0 = sysbus_create_simple("scoop", 0x10800000, NULL);
     if (model != akita) {
-        scp1 = sysbus_create_simple("scoop", 0x08800040, NULL);
+        sms->scp1 = sysbus_create_simple("scoop", 0x08800040, NULL);
+    } else {
+        sms->scp1 = NULL;
     }
 
-    spitz_scoop_gpio_setup(mpu, scp0, scp1);
+    spitz_scoop_gpio_setup(sms);
 
     spitz_gpio_setup(mpu, (model == akita) ? 1 : 2);
 
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 21/34] hw/arm/spitz: Implement inbound GPIO lines for bit5 and power signals
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (19 preceding siblings ...)
  2020-07-03 16:53 ` [PULL 20/34] hw/arm/spitz: Keep pointers to scp0, scp1 " Peter Maydell
@ 2020-07-03 16:53 ` Peter Maydell
  2020-07-03 16:53 ` [PULL 22/34] hw/misc/max111x: provide QOM properties for setting initial values Peter Maydell
                   ` (14 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:53 UTC (permalink / raw)
  To: qemu-devel

Currently the Spitz board uses a nasty hack for the GPIO lines
that pass "bit5" and "power" information to the LCD controller:
the lcdtg realize function sets a global variable to point to
the instance it just realized, and then the functions spitz_bl_power()
and spitz_bl_bit5() use that to find the device they are changing
the internal state of. There is a comment reading:
 FIXME: Implement GPIO properly and remove this hack.
which was added in 2009.

Implement GPIO properly and remove this hack.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-id: 20200628142429.17111-6-peter.maydell@linaro.org
---
 hw/arm/spitz.c | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index 69bc2b3fa10..11e413723f4 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -586,12 +586,9 @@ static void spitz_bl_update(SpitzLCDTG *s)
         zaurus_printf("LCD Backlight now off\n");
 }
 
-/* FIXME: Implement GPIO properly and remove this hack.  */
-static SpitzLCDTG *spitz_lcdtg;
-
 static inline void spitz_bl_bit5(void *opaque, int line, int level)
 {
-    SpitzLCDTG *s = spitz_lcdtg;
+    SpitzLCDTG *s = opaque;
     int prev = s->bl_intensity;
 
     if (level)
@@ -605,7 +602,7 @@ static inline void spitz_bl_bit5(void *opaque, int line, int level)
 
 static inline void spitz_bl_power(void *opaque, int line, int level)
 {
-    SpitzLCDTG *s = spitz_lcdtg;
+    SpitzLCDTG *s = opaque;
     s->bl_power = !!level;
     spitz_bl_update(s);
 }
@@ -639,13 +636,16 @@ static uint32_t spitz_lcdtg_transfer(SSISlave *dev, uint32_t value)
     return 0;
 }
 
-static void spitz_lcdtg_realize(SSISlave *dev, Error **errp)
+static void spitz_lcdtg_realize(SSISlave *ssi, Error **errp)
 {
-    SpitzLCDTG *s = FROM_SSI_SLAVE(SpitzLCDTG, dev);
+    SpitzLCDTG *s = FROM_SSI_SLAVE(SpitzLCDTG, ssi);
+    DeviceState *dev = DEVICE(s);
 
-    spitz_lcdtg = s;
     s->bl_power = 0;
     s->bl_intensity = 0x20;
+
+    qdev_init_gpio_in_named(dev, spitz_bl_bit5, "bl_bit5", 1);
+    qdev_init_gpio_in_named(dev, spitz_bl_power, "bl_power", 1);
 }
 
 /* SSP devices */
@@ -820,15 +820,11 @@ static void spitz_out_switch(void *opaque, int line, int level)
     case 3:
         zaurus_printf("Orange LED %s.\n", level ? "on" : "off");
         break;
-    case 4:
-        spitz_bl_bit5(opaque, line, level);
-        break;
-    case 5:
-        spitz_bl_power(opaque, line, level);
-        break;
     case 6:
         spitz_adc_temp_on(opaque, line, level);
         break;
+    default:
+        g_assert_not_reached();
     }
 }
 
@@ -858,9 +854,9 @@ static void spitz_scoop_gpio_setup(SpitzMachineState *sms)
 
     if (sms->scp1) {
         qdev_connect_gpio_out(sms->scp1, SPITZ_SCP2_BACKLIGHT_CONT,
-                              outsignals[4]);
+                              qdev_get_gpio_in_named(sms->lcdtg, "bl_bit5", 0));
         qdev_connect_gpio_out(sms->scp1, SPITZ_SCP2_BACKLIGHT_ON,
-                              outsignals[5]);
+                              qdev_get_gpio_in_named(sms->lcdtg, "bl_power", 0));
     }
 
     qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_ADC_TEMP_ON, outsignals[6]);
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 22/34] hw/misc/max111x: provide QOM properties for setting initial values
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (20 preceding siblings ...)
  2020-07-03 16:53 ` [PULL 21/34] hw/arm/spitz: Implement inbound GPIO lines for bit5 and power signals Peter Maydell
@ 2020-07-03 16:53 ` Peter Maydell
  2020-07-03 16:53 ` [PULL 23/34] hw/misc/max111x: Don't use vmstate_register() Peter Maydell
                   ` (13 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:53 UTC (permalink / raw)
  To: qemu-devel

Add some QOM properties to the max111x ADC device to allow the
initial values to be configured. Currently this is done by
board code calling max111x_set_input() after it creates the
device, which doesn't work on system reset.

This requires us to implement a reset method for this device,
so while we're doing that make sure we reset the other parts
of the device state.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-id: 20200628142429.17111-7-peter.maydell@linaro.org
---
 hw/misc/max111x.c | 57 ++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 47 insertions(+), 10 deletions(-)

diff --git a/hw/misc/max111x.c b/hw/misc/max111x.c
index 2b87bdee5b7..d0e5534e4f5 100644
--- a/hw/misc/max111x.c
+++ b/hw/misc/max111x.c
@@ -15,11 +15,15 @@
 #include "hw/ssi/ssi.h"
 #include "migration/vmstate.h"
 #include "qemu/module.h"
+#include "hw/qdev-properties.h"
 
 typedef struct {
     SSISlave parent_obj;
 
     qemu_irq interrupt;
+    /* Values of inputs at system reset (settable by QOM property) */
+    uint8_t reset_input[8];
+
     uint8_t tb1, rb2, rb3;
     int cycle;
 
@@ -135,16 +139,6 @@ static int max111x_init(SSISlave *d, int inputs)
     qdev_init_gpio_out(dev, &s->interrupt, 1);
 
     s->inputs = inputs;
-    /* TODO: add a user interface for setting these */
-    s->input[0] = 0xf0;
-    s->input[1] = 0xe0;
-    s->input[2] = 0xd0;
-    s->input[3] = 0xc0;
-    s->input[4] = 0xb0;
-    s->input[5] = 0xa0;
-    s->input[6] = 0x90;
-    s->input[7] = 0x80;
-    s->com = 0;
 
     vmstate_register(VMSTATE_IF(dev), VMSTATE_INSTANCE_ID_ANY,
                      &vmstate_max111x, s);
@@ -168,11 +162,50 @@ void max111x_set_input(DeviceState *dev, int line, uint8_t value)
     s->input[line] = value;
 }
 
+static void max111x_reset(DeviceState *dev)
+{
+    MAX111xState *s = MAX_111X(dev);
+    int i;
+
+    for (i = 0; i < s->inputs; i++) {
+        s->input[i] = s->reset_input[i];
+    }
+    s->com = 0;
+    s->tb1 = 0;
+    s->rb2 = 0;
+    s->rb3 = 0;
+    s->cycle = 0;
+}
+
+static Property max1110_properties[] = {
+    /* Reset values for ADC inputs */
+    DEFINE_PROP_UINT8("input0", MAX111xState, reset_input[0], 0xf0),
+    DEFINE_PROP_UINT8("input1", MAX111xState, reset_input[1], 0xe0),
+    DEFINE_PROP_UINT8("input2", MAX111xState, reset_input[2], 0xd0),
+    DEFINE_PROP_UINT8("input3", MAX111xState, reset_input[3], 0xc0),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static Property max1111_properties[] = {
+    /* Reset values for ADC inputs */
+    DEFINE_PROP_UINT8("input0", MAX111xState, reset_input[0], 0xf0),
+    DEFINE_PROP_UINT8("input1", MAX111xState, reset_input[1], 0xe0),
+    DEFINE_PROP_UINT8("input2", MAX111xState, reset_input[2], 0xd0),
+    DEFINE_PROP_UINT8("input3", MAX111xState, reset_input[3], 0xc0),
+    DEFINE_PROP_UINT8("input4", MAX111xState, reset_input[4], 0xb0),
+    DEFINE_PROP_UINT8("input5", MAX111xState, reset_input[5], 0xa0),
+    DEFINE_PROP_UINT8("input6", MAX111xState, reset_input[6], 0x90),
+    DEFINE_PROP_UINT8("input7", MAX111xState, reset_input[7], 0x80),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void max111x_class_init(ObjectClass *klass, void *data)
 {
     SSISlaveClass *k = SSI_SLAVE_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(klass);
 
     k->transfer = max111x_transfer;
+    dc->reset = max111x_reset;
 }
 
 static const TypeInfo max111x_info = {
@@ -186,8 +219,10 @@ static const TypeInfo max111x_info = {
 static void max1110_class_init(ObjectClass *klass, void *data)
 {
     SSISlaveClass *k = SSI_SLAVE_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(klass);
 
     k->realize = max1110_realize;
+    device_class_set_props(dc, max1110_properties);
 }
 
 static const TypeInfo max1110_info = {
@@ -199,8 +234,10 @@ static const TypeInfo max1110_info = {
 static void max1111_class_init(ObjectClass *klass, void *data)
 {
     SSISlaveClass *k = SSI_SLAVE_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(klass);
 
     k->realize = max1111_realize;
+    device_class_set_props(dc, max1111_properties);
 }
 
 static const TypeInfo max1111_info = {
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 23/34] hw/misc/max111x: Don't use vmstate_register()
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (21 preceding siblings ...)
  2020-07-03 16:53 ` [PULL 22/34] hw/misc/max111x: provide QOM properties for setting initial values Peter Maydell
@ 2020-07-03 16:53 ` Peter Maydell
  2020-07-03 16:53 ` [PULL 24/34] ssi: Add ssi_realize_and_unref() Peter Maydell
                   ` (12 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:53 UTC (permalink / raw)
  To: qemu-devel

The max111x is a proper qdev device; we can use dc->vmsd rather than
directly calling vmstate_register().

It's possible that this is a migration compat break, but the only
boards that use this device are the spitz-family ('akita', 'borzoi',
'spitz', 'terrier').

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-id: 20200628142429.17111-8-peter.maydell@linaro.org
---
 hw/misc/max111x.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/misc/max111x.c b/hw/misc/max111x.c
index d0e5534e4f5..abddfa3c660 100644
--- a/hw/misc/max111x.c
+++ b/hw/misc/max111x.c
@@ -140,8 +140,6 @@ static int max111x_init(SSISlave *d, int inputs)
 
     s->inputs = inputs;
 
-    vmstate_register(VMSTATE_IF(dev), VMSTATE_INSTANCE_ID_ANY,
-                     &vmstate_max111x, s);
     return 0;
 }
 
@@ -206,6 +204,7 @@ static void max111x_class_init(ObjectClass *klass, void *data)
 
     k->transfer = max111x_transfer;
     dc->reset = max111x_reset;
+    dc->vmsd = &vmstate_max111x;
 }
 
 static const TypeInfo max111x_info = {
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 24/34] ssi: Add ssi_realize_and_unref()
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (22 preceding siblings ...)
  2020-07-03 16:53 ` [PULL 23/34] hw/misc/max111x: Don't use vmstate_register() Peter Maydell
@ 2020-07-03 16:53 ` Peter Maydell
  2020-07-03 16:53 ` [PULL 25/34] hw/arm/spitz: Use max111x properties to set initial values Peter Maydell
                   ` (11 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:53 UTC (permalink / raw)
  To: qemu-devel

Add an ssi_realize_and_unref(), for the benefit of callers
who want to be able to create an SSI device, set QOM properties
on it, and then do the realize-and-unref afterwards.

The API works on the same principle as the recently added
qdev_realize_and_undef(), sysbus_realize_and_undef(), etc.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-id: 20200628142429.17111-9-peter.maydell@linaro.org
---
 include/hw/ssi/ssi.h | 26 ++++++++++++++++++++++++++
 hw/ssi/ssi.c         |  7 ++++++-
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/include/hw/ssi/ssi.h b/include/hw/ssi/ssi.h
index 93f2b8b0beb..4be5325e654 100644
--- a/include/hw/ssi/ssi.h
+++ b/include/hw/ssi/ssi.h
@@ -79,6 +79,32 @@ extern const VMStateDescription vmstate_ssi_slave;
 }
 
 DeviceState *ssi_create_slave(SSIBus *bus, const char *name);
+/**
+ * ssi_realize_and_unref: realize and unref an SSI slave device
+ * @dev: SSI slave device to realize
+ * @bus: SSI bus to put it on
+ * @errp: error pointer
+ *
+ * Call 'realize' on @dev, put it on the specified @bus, and drop the
+ * reference to it. Errors are reported via @errp and by returning
+ * false.
+ *
+ * This function is useful if you have created @dev via qdev_new()
+ * (which takes a reference to the device it returns to you), so that
+ * you can set properties on it before realizing it. If you don't need
+ * to set properties then ssi_create_slave() is probably better (as it
+ * does the create, init and realize in one step).
+ *
+ * If you are embedding the SSI slave into another QOM device and
+ * initialized it via some variant on object_initialize_child() then
+ * do not use this function, because that family of functions arrange
+ * for the only reference to the child device to be held by the parent
+ * via the child<> property, and so the reference-count-drop done here
+ * would be incorrect.  (Instead you would want ssi_realize(), which
+ * doesn't currently exist but would be trivial to create if we had
+ * any code that wanted it.)
+ */
+bool ssi_realize_and_unref(DeviceState *dev, SSIBus *bus, Error **errp);
 
 /* Master interface.  */
 SSIBus *ssi_create_bus(DeviceState *parent, const char *name);
diff --git a/hw/ssi/ssi.c b/hw/ssi/ssi.c
index 67b48c31cd6..a35d7ebb266 100644
--- a/hw/ssi/ssi.c
+++ b/hw/ssi/ssi.c
@@ -90,11 +90,16 @@ static const TypeInfo ssi_slave_info = {
     .abstract = true,
 };
 
+bool ssi_realize_and_unref(DeviceState *dev, SSIBus *bus, Error **errp)
+{
+    return qdev_realize_and_unref(dev, &bus->parent_obj, errp);
+}
+
 DeviceState *ssi_create_slave(SSIBus *bus, const char *name)
 {
     DeviceState *dev = qdev_new(name);
 
-    qdev_realize_and_unref(dev, &bus->parent_obj, &error_fatal);
+    ssi_realize_and_unref(dev, bus, &error_fatal);
     return dev;
 }
 
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 25/34] hw/arm/spitz: Use max111x properties to set initial values
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (23 preceding siblings ...)
  2020-07-03 16:53 ` [PULL 24/34] ssi: Add ssi_realize_and_unref() Peter Maydell
@ 2020-07-03 16:53 ` Peter Maydell
  2020-07-03 16:53 ` [PULL 26/34] hw/misc/max111x: Use GPIO lines rather than max111x_set_input() Peter Maydell
                   ` (10 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:53 UTC (permalink / raw)
  To: qemu-devel

Use the new max111x qdev properties to set the initial input
values rather than calling max111x_set_input(); this means that
on system reset the inputs will correctly return to their initial
values.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20200628142429.17111-10-peter.maydell@linaro.org
---
 hw/arm/spitz.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index 11e413723f4..93a25edcb5b 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -731,11 +731,14 @@ static void spitz_ssp_attach(SpitzMachineState *sms)
                           qdev_get_gpio_in(sms->mpu->gpio, SPITZ_GPIO_TP_INT));
 
     bus = qdev_get_child_bus(sms->mux, "ssi2");
-    sms->max1111 = ssi_create_slave(bus, "max1111");
+    sms->max1111 = qdev_new("max1111");
     max1111 = sms->max1111;
-    max111x_set_input(sms->max1111, MAX1111_BATT_VOLT, SPITZ_BATTERY_VOLT);
-    max111x_set_input(sms->max1111, MAX1111_BATT_TEMP, 0);
-    max111x_set_input(sms->max1111, MAX1111_ACIN_VOLT, SPITZ_CHARGEON_ACIN);
+    qdev_prop_set_uint8(sms->max1111, "input1" /* BATT_VOLT */,
+                        SPITZ_BATTERY_VOLT);
+    qdev_prop_set_uint8(sms->max1111, "input2" /* BATT_TEMP */, 0);
+    qdev_prop_set_uint8(sms->max1111, "input3" /* ACIN_VOLT */,
+                        SPITZ_CHARGEON_ACIN);
+    ssi_realize_and_unref(sms->max1111, bus, &error_fatal);
 
     qdev_connect_gpio_out(sms->mpu->gpio, SPITZ_GPIO_LCDCON_CS,
                         qdev_get_gpio_in(sms->mux, 0));
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 26/34] hw/misc/max111x: Use GPIO lines rather than max111x_set_input()
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (24 preceding siblings ...)
  2020-07-03 16:53 ` [PULL 25/34] hw/arm/spitz: Use max111x properties to set initial values Peter Maydell
@ 2020-07-03 16:53 ` Peter Maydell
  2020-07-03 16:53 ` [PULL 27/34] hw/misc/max111x: Create header file for documentation, TYPE_ macros Peter Maydell
                   ` (9 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:53 UTC (permalink / raw)
  To: qemu-devel

The max111x ADC device model allows other code to set the level on
the 8 ADC inputs using the max111x_set_input() function.  Replace
this with generic qdev GPIO inputs, which also allow inputs to be set
to arbitrary values.

Using GPIO lines will make it easier for board code to wire things
up, so that if device A wants to set the ADC input it doesn't need to
have a direct pointer to the max111x but can just set that value on
its output GPIO, which is then wired up by the board to the
appropriate max111x input.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-id: 20200628142429.17111-11-peter.maydell@linaro.org
---
 include/hw/ssi/ssi.h |  3 ---
 hw/arm/spitz.c       |  9 +++++----
 hw/misc/max111x.c    | 16 +++++++++-------
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/include/hw/ssi/ssi.h b/include/hw/ssi/ssi.h
index 4be5325e654..5fd411f2e4e 100644
--- a/include/hw/ssi/ssi.h
+++ b/include/hw/ssi/ssi.h
@@ -111,7 +111,4 @@ SSIBus *ssi_create_bus(DeviceState *parent, const char *name);
 
 uint32_t ssi_transfer(SSIBus *bus, uint32_t val);
 
-/* max111x.c */
-void max111x_set_input(DeviceState *dev, int line, uint8_t value);
-
 #endif
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index 93a25edcb5b..fa592aad6d6 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -696,13 +696,14 @@ static void corgi_ssp_gpio_cs(void *opaque, int line, int level)
 
 static void spitz_adc_temp_on(void *opaque, int line, int level)
 {
+    int batt_temp;
+
     if (!max1111)
         return;
 
-    if (level)
-        max111x_set_input(max1111, MAX1111_BATT_TEMP, SPITZ_BATTERY_TEMP);
-    else
-        max111x_set_input(max1111, MAX1111_BATT_TEMP, 0);
+    batt_temp = level ? SPITZ_BATTERY_TEMP : 0;
+
+    qemu_set_irq(qdev_get_gpio_in(max1111, MAX1111_BATT_TEMP), batt_temp);
 }
 
 static void corgi_ssp_realize(SSISlave *d, Error **errp)
diff --git a/hw/misc/max111x.c b/hw/misc/max111x.c
index abddfa3c660..3a5cb838445 100644
--- a/hw/misc/max111x.c
+++ b/hw/misc/max111x.c
@@ -131,12 +131,21 @@ static const VMStateDescription vmstate_max111x = {
     }
 };
 
+static void max111x_input_set(void *opaque, int line, int value)
+{
+    MAX111xState *s = MAX_111X(opaque);
+
+    assert(line >= 0 && line < s->inputs);
+    s->input[line] = value;
+}
+
 static int max111x_init(SSISlave *d, int inputs)
 {
     DeviceState *dev = DEVICE(d);
     MAX111xState *s = MAX_111X(dev);
 
     qdev_init_gpio_out(dev, &s->interrupt, 1);
+    qdev_init_gpio_in(dev, max111x_input_set, inputs);
 
     s->inputs = inputs;
 
@@ -153,13 +162,6 @@ static void max1111_realize(SSISlave *dev, Error **errp)
     max111x_init(dev, 4);
 }
 
-void max111x_set_input(DeviceState *dev, int line, uint8_t value)
-{
-    MAX111xState *s = MAX_111X(dev);
-    assert(line >= 0 && line < s->inputs);
-    s->input[line] = value;
-}
-
 static void max111x_reset(DeviceState *dev)
 {
     MAX111xState *s = MAX_111X(dev);
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 27/34] hw/misc/max111x: Create header file for documentation, TYPE_ macros
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (25 preceding siblings ...)
  2020-07-03 16:53 ` [PULL 26/34] hw/misc/max111x: Use GPIO lines rather than max111x_set_input() Peter Maydell
@ 2020-07-03 16:53 ` Peter Maydell
  2020-07-03 16:53 ` [PULL 28/34] hw/arm/spitz: Encapsulate misc GPIO handling in a device Peter Maydell
                   ` (8 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:53 UTC (permalink / raw)
  To: qemu-devel

Create a header file for the hw/misc/max111x device, in the
usual modern style for QOM devices:
 * definition of the TYPE_ constants and macros
 * definition of the device's state struct so that it can
   be embedded in other structs if desired
 * documentation of the interface

This allows us to use TYPE_MAX_1111 in the spitz.c code rather
than the string "max1111".

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20200628142429.17111-12-peter.maydell@linaro.org
---
 include/hw/misc/max111x.h | 56 +++++++++++++++++++++++++++++++++++++++
 hw/arm/spitz.c            |  3 ++-
 hw/misc/max111x.c         | 24 +----------------
 MAINTAINERS               |  1 +
 4 files changed, 60 insertions(+), 24 deletions(-)
 create mode 100644 include/hw/misc/max111x.h

diff --git a/include/hw/misc/max111x.h b/include/hw/misc/max111x.h
new file mode 100644
index 00000000000..af7f1017eff
--- /dev/null
+++ b/include/hw/misc/max111x.h
@@ -0,0 +1,56 @@
+/*
+ * Maxim MAX1110/1111 ADC chip emulation.
+ *
+ * Copyright (c) 2006 Openedhand Ltd.
+ * Written by Andrzej Zaborowski <balrog@zabor.org>
+ *
+ * This code is licensed under the GNU GPLv2.
+ *
+ * Contributions after 2012-01-13 are licensed under the terms of the
+ * GNU GPL, version 2 or (at your option) any later version.
+ */
+
+#ifndef HW_MISC_MAX111X_H
+#define HW_MISC_MAX111X_H
+
+#include "hw/ssi/ssi.h"
+
+/*
+ * This is a model of the Maxim MAX1110/1111 ADC chip, which for QEMU
+ * is an SSI slave device. It has either 4 (max1110) or 8 (max1111)
+ * 8-bit ADC channels.
+ *
+ * QEMU interface:
+ *  + GPIO inputs 0..3 (for max1110) or 0..7 (for max1111): set the value
+ *    of each ADC input, as an unsigned 8-bit value
+ *  + GPIO output 0: interrupt line
+ *  + Properties "input0" to "input3" (max1110) or "input0" to "input7"
+ *    (max1111): initial reset values for ADC inputs.
+ *
+ * Known bugs:
+ *  + the interrupt line is not correctly implemented, and will never
+ *    be lowered once it has been asserted.
+ */
+typedef struct {
+    SSISlave parent_obj;
+
+    qemu_irq interrupt;
+    /* Values of inputs at system reset (settable by QOM property) */
+    uint8_t reset_input[8];
+
+    uint8_t tb1, rb2, rb3;
+    int cycle;
+
+    uint8_t input[8];
+    int inputs, com;
+} MAX111xState;
+
+#define TYPE_MAX_111X "max111x"
+
+#define MAX_111X(obj) \
+    OBJECT_CHECK(MAX111xState, (obj), TYPE_MAX_111X)
+
+#define TYPE_MAX_1110 "max1110"
+#define TYPE_MAX_1111 "max1111"
+
+#endif
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index fa592aad6d6..1400a56729d 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -29,6 +29,7 @@
 #include "audio/audio.h"
 #include "hw/boards.h"
 #include "hw/sysbus.h"
+#include "hw/misc/max111x.h"
 #include "migration/vmstate.h"
 #include "exec/address-spaces.h"
 #include "cpu.h"
@@ -732,7 +733,7 @@ static void spitz_ssp_attach(SpitzMachineState *sms)
                           qdev_get_gpio_in(sms->mpu->gpio, SPITZ_GPIO_TP_INT));
 
     bus = qdev_get_child_bus(sms->mux, "ssi2");
-    sms->max1111 = qdev_new("max1111");
+    sms->max1111 = qdev_new(TYPE_MAX_1111);
     max1111 = sms->max1111;
     qdev_prop_set_uint8(sms->max1111, "input1" /* BATT_VOLT */,
                         SPITZ_BATTERY_VOLT);
diff --git a/hw/misc/max111x.c b/hw/misc/max111x.c
index 3a5cb838445..7e6723f3435 100644
--- a/hw/misc/max111x.c
+++ b/hw/misc/max111x.c
@@ -11,34 +11,12 @@
  */
 
 #include "qemu/osdep.h"
+#include "hw/misc/max111x.h"
 #include "hw/irq.h"
-#include "hw/ssi/ssi.h"
 #include "migration/vmstate.h"
 #include "qemu/module.h"
 #include "hw/qdev-properties.h"
 
-typedef struct {
-    SSISlave parent_obj;
-
-    qemu_irq interrupt;
-    /* Values of inputs at system reset (settable by QOM property) */
-    uint8_t reset_input[8];
-
-    uint8_t tb1, rb2, rb3;
-    int cycle;
-
-    uint8_t input[8];
-    int inputs, com;
-} MAX111xState;
-
-#define TYPE_MAX_111X "max111x"
-
-#define MAX_111X(obj) \
-    OBJECT_CHECK(MAX111xState, (obj), TYPE_MAX_111X)
-
-#define TYPE_MAX_1110 "max1110"
-#define TYPE_MAX_1111 "max1111"
-
 /* Control-byte bitfields */
 #define CB_PD0		(1 << 0)
 #define CB_PD1		(1 << 1)
diff --git a/MAINTAINERS b/MAINTAINERS
index dec252f38b1..c31c878c635 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -787,6 +787,7 @@ F: hw/gpio/max7310.c
 F: hw/gpio/zaurus.c
 F: hw/misc/mst_fpga.c
 F: hw/misc/max111x.c
+F: include/hw/misc/max111x.h
 F: include/hw/arm/pxa.h
 F: include/hw/arm/sharpsl.h
 F: include/hw/display/tc6393xb.h
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 28/34] hw/arm/spitz: Encapsulate misc GPIO handling in a device
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (26 preceding siblings ...)
  2020-07-03 16:53 ` [PULL 27/34] hw/misc/max111x: Create header file for documentation, TYPE_ macros Peter Maydell
@ 2020-07-03 16:53 ` Peter Maydell
  2020-07-03 16:54 ` [PULL 29/34] hw/gpio/zaurus.c: Use LOG_GUEST_ERROR for bad guest register accesses Peter Maydell
                   ` (7 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:53 UTC (permalink / raw)
  To: qemu-devel

Currently we have a free-floating set of IRQs and a function
spitz_out_switch() which handle some miscellaneous GPIO lines for the
spitz board.  Encapsulate this behaviour in a simple QOM device.

At this point we can finally remove the 'max1111' global, because the
ADC battery-temperature value is now handled by the misc-gpio device
writing the value to its outbound "adc-temp" GPIO, which the board
code wires up to the appropriate inbound GPIO line on the max1111.

This commit also fixes Coverity issue CID 1421913 (which pointed out
that the 'outsignals' in spitz_scoop_gpio_setup() were leaked),
because it removes the use of the qemu_allocate_irqs() API from this
code entirely.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-id: 20200628142429.17111-13-peter.maydell@linaro.org
---
 hw/arm/spitz.c | 129 +++++++++++++++++++++++++++++++++----------------
 1 file changed, 87 insertions(+), 42 deletions(-)

diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index 1400a56729d..bab9968ccee 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -51,6 +51,7 @@ typedef struct {
     DeviceState *max1111;
     DeviceState *scp0;
     DeviceState *scp1;
+    DeviceState *misc_gpio;
 } SpitzMachineState;
 
 #define TYPE_SPITZ_MACHINE "spitz-common"
@@ -658,8 +659,6 @@ static void spitz_lcdtg_realize(SSISlave *ssi, Error **errp)
 #define SPITZ_GPIO_MAX1111_CS   20
 #define SPITZ_GPIO_TP_INT       11
 
-static DeviceState *max1111;
-
 /* "Demux" the signal based on current chipselect */
 typedef struct {
     SSISlave ssidev;
@@ -695,18 +694,6 @@ static void corgi_ssp_gpio_cs(void *opaque, int line, int level)
 #define SPITZ_BATTERY_VOLT      0xd0    /* About 4.0V */
 #define SPITZ_CHARGEON_ACIN     0x80    /* About 5.0V */
 
-static void spitz_adc_temp_on(void *opaque, int line, int level)
-{
-    int batt_temp;
-
-    if (!max1111)
-        return;
-
-    batt_temp = level ? SPITZ_BATTERY_TEMP : 0;
-
-    qemu_set_irq(qdev_get_gpio_in(max1111, MAX1111_BATT_TEMP), batt_temp);
-}
-
 static void corgi_ssp_realize(SSISlave *d, Error **errp)
 {
     DeviceState *dev = DEVICE(d);
@@ -734,7 +721,6 @@ static void spitz_ssp_attach(SpitzMachineState *sms)
 
     bus = qdev_get_child_bus(sms->mux, "ssi2");
     sms->max1111 = qdev_new(TYPE_MAX_1111);
-    max1111 = sms->max1111;
     qdev_prop_set_uint8(sms->max1111, "input1" /* BATT_VOLT */,
                         SPITZ_BATTERY_VOLT);
     qdev_prop_set_uint8(sms->max1111, "input2" /* BATT_TEMP */, 0);
@@ -810,27 +796,66 @@ static void spitz_akita_i2c_setup(PXA2xxState *cpu)
 
 /* Other peripherals */
 
-static void spitz_out_switch(void *opaque, int line, int level)
+/*
+ * Encapsulation of some miscellaneous GPIO line behaviour for the Spitz boards.
+ *
+ * QEMU interface:
+ *  + named GPIO inputs "green-led", "orange-led", "charging", "discharging":
+ *    these currently just print messages that the line has been signalled
+ *  + named GPIO input "adc-temp-on": set to cause the battery-temperature
+ *    value to be passed to the max111x ADC
+ *  + named GPIO output "adc-temp": the ADC value, to be wired up to the max111x
+ */
+#define TYPE_SPITZ_MISC_GPIO "spitz-misc-gpio"
+#define SPITZ_MISC_GPIO(obj) \
+    OBJECT_CHECK(SpitzMiscGPIOState, (obj), TYPE_SPITZ_MISC_GPIO)
+
+typedef struct SpitzMiscGPIOState {
+    SysBusDevice parent_obj;
+
+    qemu_irq adc_value;
+} SpitzMiscGPIOState;
+
+static void spitz_misc_charging(void *opaque, int n, int level)
 {
-    switch (line) {
-    case 0:
-        zaurus_printf("Charging %s.\n", level ? "off" : "on");
-        break;
-    case 1:
-        zaurus_printf("Discharging %s.\n", level ? "on" : "off");
-        break;
-    case 2:
-        zaurus_printf("Green LED %s.\n", level ? "on" : "off");
-        break;
-    case 3:
-        zaurus_printf("Orange LED %s.\n", level ? "on" : "off");
-        break;
-    case 6:
-        spitz_adc_temp_on(opaque, line, level);
-        break;
-    default:
-        g_assert_not_reached();
-    }
+    zaurus_printf("Charging %s.\n", level ? "off" : "on");
+}
+
+static void spitz_misc_discharging(void *opaque, int n, int level)
+{
+    zaurus_printf("Discharging %s.\n", level ? "off" : "on");
+}
+
+static void spitz_misc_green_led(void *opaque, int n, int level)
+{
+    zaurus_printf("Green LED %s.\n", level ? "off" : "on");
+}
+
+static void spitz_misc_orange_led(void *opaque, int n, int level)
+{
+    zaurus_printf("Orange LED %s.\n", level ? "off" : "on");
+}
+
+static void spitz_misc_adc_temp(void *opaque, int n, int level)
+{
+    SpitzMiscGPIOState *s = SPITZ_MISC_GPIO(opaque);
+    int batt_temp = level ? SPITZ_BATTERY_TEMP : 0;
+
+    qemu_set_irq(s->adc_value, batt_temp);
+}
+
+static void spitz_misc_gpio_init(Object *obj)
+{
+    SpitzMiscGPIOState *s = SPITZ_MISC_GPIO(obj);
+    DeviceState *dev = DEVICE(obj);
+
+    qdev_init_gpio_in_named(dev, spitz_misc_charging, "charging", 1);
+    qdev_init_gpio_in_named(dev, spitz_misc_discharging, "discharging", 1);
+    qdev_init_gpio_in_named(dev, spitz_misc_green_led, "green-led", 1);
+    qdev_init_gpio_in_named(dev, spitz_misc_orange_led, "orange-led", 1);
+    qdev_init_gpio_in_named(dev, spitz_misc_adc_temp, "adc-temp-on", 1);
+
+    qdev_init_gpio_out_named(dev, &s->adc_value, "adc-temp", 1);
 }
 
 #define SPITZ_SCP_LED_GREEN             1
@@ -850,12 +875,22 @@ static void spitz_out_switch(void *opaque, int line, int level)
 
 static void spitz_scoop_gpio_setup(SpitzMachineState *sms)
 {
-    qemu_irq *outsignals = qemu_allocate_irqs(spitz_out_switch, sms->mpu, 8);
+    DeviceState *miscdev = sysbus_create_simple(TYPE_SPITZ_MISC_GPIO, -1, NULL);
 
-    qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_CHRG_ON, outsignals[0]);
-    qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_JK_B, outsignals[1]);
-    qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_LED_GREEN, outsignals[2]);
-    qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_LED_ORANGE, outsignals[3]);
+    sms->misc_gpio = miscdev;
+
+    qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_CHRG_ON,
+                          qdev_get_gpio_in_named(miscdev, "charging", 0));
+    qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_JK_B,
+                          qdev_get_gpio_in_named(miscdev, "discharging", 0));
+    qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_LED_GREEN,
+                          qdev_get_gpio_in_named(miscdev, "green-led", 0));
+    qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_LED_ORANGE,
+                          qdev_get_gpio_in_named(miscdev, "orange-led", 0));
+    qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_ADC_TEMP_ON,
+                          qdev_get_gpio_in_named(miscdev, "adc-temp-on", 0));
+    qdev_connect_gpio_out_named(miscdev, "adc-temp", 0,
+                                qdev_get_gpio_in(sms->max1111, MAX1111_BATT_TEMP));
 
     if (sms->scp1) {
         qdev_connect_gpio_out(sms->scp1, SPITZ_SCP2_BACKLIGHT_CONT,
@@ -863,8 +898,6 @@ static void spitz_scoop_gpio_setup(SpitzMachineState *sms)
         qdev_connect_gpio_out(sms->scp1, SPITZ_SCP2_BACKLIGHT_ON,
                               qdev_get_gpio_in_named(sms->lcdtg, "bl_power", 0));
     }
-
-    qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_ADC_TEMP_ON, outsignals[6]);
 }
 
 #define SPITZ_GPIO_HSYNC                22
@@ -1217,12 +1250,24 @@ static const TypeInfo spitz_lcdtg_info = {
     .class_init    = spitz_lcdtg_class_init,
 };
 
+static const TypeInfo spitz_misc_gpio_info = {
+    .name = TYPE_SPITZ_MISC_GPIO,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(SpitzMiscGPIOState),
+    .instance_init = spitz_misc_gpio_init,
+    /*
+     * No class_init required: device has no internal state so does not
+     * need to set up reset or vmstate, and does not have a realize method.
+     */
+};
+
 static void spitz_register_types(void)
 {
     type_register_static(&corgi_ssp_info);
     type_register_static(&spitz_lcdtg_info);
     type_register_static(&spitz_keyboard_info);
     type_register_static(&sl_nand_info);
+    type_register_static(&spitz_misc_gpio_info);
 }
 
 type_init(spitz_register_types)
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 29/34] hw/gpio/zaurus.c: Use LOG_GUEST_ERROR for bad guest register accesses
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (27 preceding siblings ...)
  2020-07-03 16:53 ` [PULL 28/34] hw/arm/spitz: Encapsulate misc GPIO handling in a device Peter Maydell
@ 2020-07-03 16:54 ` Peter Maydell
  2020-07-03 16:54 ` [PULL 30/34] hw/arm/spitz: " Peter Maydell
                   ` (6 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:54 UTC (permalink / raw)
  To: qemu-devel

Instead of logging guest accesses to invalid register offsets in this
device using zaurus_printf() (which just prints to stderr), use the
usual qemu_log_mask(LOG_GUEST_ERROR,...).

Since this was the only use of the zaurus_printf() macro outside
spitz.c, we can move the definition of that macro from sharpsl.h
to spitz.c.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-id: 20200628142429.17111-14-peter.maydell@linaro.org
---
 include/hw/arm/sharpsl.h |  3 ---
 hw/arm/spitz.c           |  3 +++
 hw/gpio/zaurus.c         | 12 +++++++-----
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/include/hw/arm/sharpsl.h b/include/hw/arm/sharpsl.h
index 89e168fbff3..e986b28c527 100644
--- a/include/hw/arm/sharpsl.h
+++ b/include/hw/arm/sharpsl.h
@@ -9,9 +9,6 @@
 
 #include "exec/hwaddr.h"
 
-#define zaurus_printf(format, ...)	\
-    fprintf(stderr, "%s: " format, __func__, ##__VA_ARGS__)
-
 /* zaurus.c */
 
 #define SL_PXA_PARAM_BASE	0xa0000a00
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index bab9968ccee..6eb46869157 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -62,6 +62,9 @@ typedef struct {
 #define SPITZ_MACHINE_CLASS(klass) \
     OBJECT_CLASS_CHECK(SpitzMachineClass, klass, TYPE_SPITZ_MACHINE)
 
+#define zaurus_printf(format, ...)                              \
+    fprintf(stderr, "%s: " format, __func__, ##__VA_ARGS__)
+
 #undef REG_FMT
 #define REG_FMT                         "0x%02lx"
 
diff --git a/hw/gpio/zaurus.c b/hw/gpio/zaurus.c
index 9a12c683420..258e9264930 100644
--- a/hw/gpio/zaurus.c
+++ b/hw/gpio/zaurus.c
@@ -22,9 +22,7 @@
 #include "hw/sysbus.h"
 #include "migration/vmstate.h"
 #include "qemu/module.h"
-
-#undef REG_FMT
-#define REG_FMT			"0x%02lx"
+#include "qemu/log.h"
 
 /* SCOOP devices */
 
@@ -104,7 +102,9 @@ static uint64_t scoop_read(void *opaque, hwaddr addr,
     case SCOOP_GPRR:
         return s->gpio_level;
     default:
-        zaurus_printf("Bad register offset " REG_FMT "\n", (unsigned long)addr);
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "scoop_read: bad register offset 0x%02" HWADDR_PRIx "\n",
+                      addr);
     }
 
     return 0;
@@ -150,7 +150,9 @@ static void scoop_write(void *opaque, hwaddr addr,
         scoop_gpio_handler_update(s);
         break;
     default:
-        zaurus_printf("Bad register offset " REG_FMT "\n", (unsigned long)addr);
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "scoop_write: bad register offset 0x%02" HWADDR_PRIx "\n",
+                      addr);
     }
 }
 
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 30/34] hw/arm/spitz: Use LOG_GUEST_ERROR for bad guest register accesses
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (28 preceding siblings ...)
  2020-07-03 16:54 ` [PULL 29/34] hw/gpio/zaurus.c: Use LOG_GUEST_ERROR for bad guest register accesses Peter Maydell
@ 2020-07-03 16:54 ` Peter Maydell
  2020-07-03 16:54 ` [PULL 31/34] hw/arm/pxa2xx_pic: " Peter Maydell
                   ` (5 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:54 UTC (permalink / raw)
  To: qemu-devel

Instead of logging guest accesses to invalid register offsets in the
Spitz flash device with zaurus_printf() (which just prints to stderr),
use the usual qemu_log_mask(LOG_GUEST_ERROR,...).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-id: 20200628142429.17111-15-peter.maydell@linaro.org
---
 hw/arm/spitz.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index 6eb46869157..49eae3fce4e 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -23,6 +23,7 @@
 #include "hw/ssi/ssi.h"
 #include "hw/block/flash.h"
 #include "qemu/timer.h"
+#include "qemu/log.h"
 #include "hw/arm/sharpsl.h"
 #include "ui/console.h"
 #include "hw/audio/wm8750.h"
@@ -65,9 +66,6 @@ typedef struct {
 #define zaurus_printf(format, ...)                              \
     fprintf(stderr, "%s: " format, __func__, ##__VA_ARGS__)
 
-#undef REG_FMT
-#define REG_FMT                         "0x%02lx"
-
 /* Spitz Flash */
 #define FLASH_BASE              0x0c000000
 #define FLASH_ECCLPLB           0x00    /* Line parity 7 - 0 bit */
@@ -137,7 +135,9 @@ static uint64_t sl_read(void *opaque, hwaddr addr, unsigned size)
         return ecc_digest(&s->ecc, nand_getio(s->nand));
 
     default:
-        zaurus_printf("Bad register offset " REG_FMT "\n", (unsigned long)addr);
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "sl_read: bad register offset 0x%02" HWADDR_PRIx "\n",
+                      addr);
     }
     return 0;
 }
@@ -168,7 +168,9 @@ static void sl_write(void *opaque, hwaddr addr,
         break;
 
     default:
-        zaurus_printf("Bad register offset " REG_FMT "\n", (unsigned long)addr);
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "sl_write: bad register offset 0x%02" HWADDR_PRIx "\n",
+                      addr);
     }
 }
 
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 31/34] hw/arm/pxa2xx_pic: Use LOG_GUEST_ERROR for bad guest register accesses
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (29 preceding siblings ...)
  2020-07-03 16:54 ` [PULL 30/34] hw/arm/spitz: " Peter Maydell
@ 2020-07-03 16:54 ` Peter Maydell
  2020-07-03 16:54 ` [PULL 32/34] hw/arm/spitz: Provide usual QOM macros for corgi-ssp and spitz-lcdtg Peter Maydell
                   ` (4 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:54 UTC (permalink / raw)
  To: qemu-devel

Instead of using printf() for logging guest accesses to invalid
register offsets in the pxa2xx PIC device, use the usual
qemu_log_mask(LOG_GUEST_ERROR,...).

This was the only user of the REG_FMT macro in pxa.h, so we can
remove that.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-id: 20200628142429.17111-16-peter.maydell@linaro.org
---
 include/hw/arm/pxa.h | 1 -
 hw/arm/pxa2xx_pic.c  | 9 +++++++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/include/hw/arm/pxa.h b/include/hw/arm/pxa.h
index f6dfb5c0cf0..8843e5f9107 100644
--- a/include/hw/arm/pxa.h
+++ b/include/hw/arm/pxa.h
@@ -184,7 +184,6 @@ struct PXA2xxI2SState {
 };
 
 # define PA_FMT			"0x%08lx"
-# define REG_FMT		"0x" TARGET_FMT_plx
 
 PXA2xxState *pxa270_init(MemoryRegion *address_space, unsigned int sdram_size,
                          const char *revision);
diff --git a/hw/arm/pxa2xx_pic.c b/hw/arm/pxa2xx_pic.c
index 105c5e63f2f..ceee6aa48db 100644
--- a/hw/arm/pxa2xx_pic.c
+++ b/hw/arm/pxa2xx_pic.c
@@ -11,6 +11,7 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/module.h"
+#include "qemu/log.h"
 #include "cpu.h"
 #include "hw/arm/pxa.h"
 #include "hw/sysbus.h"
@@ -166,7 +167,9 @@ static uint64_t pxa2xx_pic_mem_read(void *opaque, hwaddr offset,
     case ICHP:	/* Highest Priority register */
         return pxa2xx_pic_highest(s);
     default:
-        printf("%s: Bad register offset " REG_FMT "\n", __func__, offset);
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "pxa2xx_pic_mem_read: bad register offset 0x%" HWADDR_PRIx
+                      "\n", offset);
         return 0;
     }
 }
@@ -199,7 +202,9 @@ static void pxa2xx_pic_mem_write(void *opaque, hwaddr offset,
         s->priority[32 + ((offset - IPR32) >> 2)] = value & 0x8000003f;
         break;
     default:
-        printf("%s: Bad register offset " REG_FMT "\n", __func__, offset);
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "pxa2xx_pic_mem_write: bad register offset 0x%"
+                      HWADDR_PRIx "\n", offset);
         return;
     }
     pxa2xx_pic_update(opaque);
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 32/34] hw/arm/spitz: Provide usual QOM macros for corgi-ssp and spitz-lcdtg
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (30 preceding siblings ...)
  2020-07-03 16:54 ` [PULL 31/34] hw/arm/pxa2xx_pic: " Peter Maydell
@ 2020-07-03 16:54 ` Peter Maydell
  2020-07-03 16:54 ` [PULL 33/34] Replace uses of FROM_SSI_SLAVE() macro with QOM casts Peter Maydell
                   ` (3 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:54 UTC (permalink / raw)
  To: qemu-devel

The QOM types "spitz-lcdtg" and "corgi-ssp" are missing the
usual QOM TYPE and casting macros; provide and use them.

In particular, we can safely use the QOM cast macros instead of
FROM_SSI_SLAVE() because in both cases the 'ssidev' field of
the instance state struct is the first field in it.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-id: 20200628142429.17111-17-peter.maydell@linaro.org
---
 hw/arm/spitz.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index 49eae3fce4e..f020aff9747 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -579,6 +579,9 @@ static void spitz_keyboard_realize(DeviceState *dev, Error **errp)
 #define LCDTG_PICTRL    0x06
 #define LCDTG_POLCTRL   0x07
 
+#define TYPE_SPITZ_LCDTG "spitz-lcdtg"
+#define SPITZ_LCDTG(obj) OBJECT_CHECK(SpitzLCDTG, (obj), TYPE_SPITZ_LCDTG)
+
 typedef struct {
     SSISlave ssidev;
     uint32_t bl_intensity;
@@ -616,7 +619,7 @@ static inline void spitz_bl_power(void *opaque, int line, int level)
 
 static uint32_t spitz_lcdtg_transfer(SSISlave *dev, uint32_t value)
 {
-    SpitzLCDTG *s = FROM_SSI_SLAVE(SpitzLCDTG, dev);
+    SpitzLCDTG *s = SPITZ_LCDTG(dev);
     int addr;
     addr = value >> 5;
     value &= 0x1f;
@@ -645,7 +648,7 @@ static uint32_t spitz_lcdtg_transfer(SSISlave *dev, uint32_t value)
 
 static void spitz_lcdtg_realize(SSISlave *ssi, Error **errp)
 {
-    SpitzLCDTG *s = FROM_SSI_SLAVE(SpitzLCDTG, ssi);
+    SpitzLCDTG *s = SPITZ_LCDTG(ssi);
     DeviceState *dev = DEVICE(s);
 
     s->bl_power = 0;
@@ -664,6 +667,9 @@ static void spitz_lcdtg_realize(SSISlave *ssi, Error **errp)
 #define SPITZ_GPIO_MAX1111_CS   20
 #define SPITZ_GPIO_TP_INT       11
 
+#define TYPE_CORGI_SSP "corgi-ssp"
+#define CORGI_SSP(obj) OBJECT_CHECK(CorgiSSPState, (obj), TYPE_CORGI_SSP)
+
 /* "Demux" the signal based on current chipselect */
 typedef struct {
     SSISlave ssidev;
@@ -673,7 +679,7 @@ typedef struct {
 
 static uint32_t corgi_ssp_transfer(SSISlave *dev, uint32_t value)
 {
-    CorgiSSPState *s = FROM_SSI_SLAVE(CorgiSSPState, dev);
+    CorgiSSPState *s = CORGI_SSP(dev);
     int i;
 
     for (i = 0; i < 3; i++) {
@@ -702,7 +708,7 @@ static void corgi_ssp_gpio_cs(void *opaque, int line, int level)
 static void corgi_ssp_realize(SSISlave *d, Error **errp)
 {
     DeviceState *dev = DEVICE(d);
-    CorgiSSPState *s = FROM_SSI_SLAVE(CorgiSSPState, d);
+    CorgiSSPState *s = CORGI_SSP(d);
 
     qdev_init_gpio_in(dev, corgi_ssp_gpio_cs, 3);
     s->bus[0] = ssi_create_bus(dev, "ssi0");
@@ -714,10 +720,11 @@ static void spitz_ssp_attach(SpitzMachineState *sms)
 {
     void *bus;
 
-    sms->mux = ssi_create_slave(sms->mpu->ssp[CORGI_SSP_PORT - 1], "corgi-ssp");
+    sms->mux = ssi_create_slave(sms->mpu->ssp[CORGI_SSP_PORT - 1],
+                                TYPE_CORGI_SSP);
 
     bus = qdev_get_child_bus(sms->mux, "ssi0");
-    sms->lcdtg = ssi_create_slave(bus, "spitz-lcdtg");
+    sms->lcdtg = ssi_create_slave(bus, TYPE_SPITZ_LCDTG);
 
     bus = qdev_get_child_bus(sms->mux, "ssi1");
     sms->ads7846 = ssi_create_slave(bus, "ads7846");
@@ -1220,7 +1227,7 @@ static void corgi_ssp_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo corgi_ssp_info = {
-    .name          = "corgi-ssp",
+    .name          = TYPE_CORGI_SSP,
     .parent        = TYPE_SSI_SLAVE,
     .instance_size = sizeof(CorgiSSPState),
     .class_init    = corgi_ssp_class_init,
@@ -1249,7 +1256,7 @@ static void spitz_lcdtg_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo spitz_lcdtg_info = {
-    .name          = "spitz-lcdtg",
+    .name          = TYPE_SPITZ_LCDTG,
     .parent        = TYPE_SSI_SLAVE,
     .instance_size = sizeof(SpitzLCDTG),
     .class_init    = spitz_lcdtg_class_init,
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 33/34] Replace uses of FROM_SSI_SLAVE() macro with QOM casts
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (31 preceding siblings ...)
  2020-07-03 16:54 ` [PULL 32/34] hw/arm/spitz: Provide usual QOM macros for corgi-ssp and spitz-lcdtg Peter Maydell
@ 2020-07-03 16:54 ` Peter Maydell
  2020-07-03 16:54 ` [PULL 34/34] Deprecate TileGX port Peter Maydell
                   ` (2 subsequent siblings)
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:54 UTC (permalink / raw)
  To: qemu-devel

The FROM_SSI_SLAVE() macro predates QOM and is used as a typesafe way
to cast from an SSISlave* to the instance struct of a subtype of
TYPE_SSI_SLAVE.  Switch to using the QOM cast macros instead, which
have the same effect (by writing the QOM macros if the types were
previously missing them.)

(The FROM_SSI_SLAVE() macro allows the SSISlave member of the
subtype's struct to be anywhere as long as it is named "ssidev",
whereas a QOM cast macro insists that it is the first thing in the
subtype's struct.  This is true for all the types we convert here.)

This removes all the uses of FROM_SSI_SLAVE() so we can delete the
definition.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-id: 20200628142429.17111-18-peter.maydell@linaro.org
---
 include/hw/ssi/ssi.h |  2 --
 hw/arm/z2.c          | 11 +++++++----
 hw/display/ads7846.c |  9 ++++++---
 hw/display/ssd0323.c | 10 +++++++---
 hw/sd/ssi-sd.c       |  4 ++--
 5 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/include/hw/ssi/ssi.h b/include/hw/ssi/ssi.h
index 5fd411f2e4e..eac168aa1db 100644
--- a/include/hw/ssi/ssi.h
+++ b/include/hw/ssi/ssi.h
@@ -66,8 +66,6 @@ struct SSISlave {
     bool cs;
 };
 
-#define FROM_SSI_SLAVE(type, dev) DO_UPCAST(type, ssidev, dev)
-
 extern const VMStateDescription vmstate_ssi_slave;
 
 #define VMSTATE_SSI_SLAVE(_field, _state) {                          \
diff --git a/hw/arm/z2.c b/hw/arm/z2.c
index a0f40959904..e1f22f58681 100644
--- a/hw/arm/z2.c
+++ b/hw/arm/z2.c
@@ -111,9 +111,12 @@ typedef struct {
     int pos;
 } ZipitLCD;
 
+#define TYPE_ZIPIT_LCD "zipit-lcd"
+#define ZIPIT_LCD(obj) OBJECT_CHECK(ZipitLCD, (obj), TYPE_ZIPIT_LCD)
+
 static uint32_t zipit_lcd_transfer(SSISlave *dev, uint32_t value)
 {
-    ZipitLCD *z = FROM_SSI_SLAVE(ZipitLCD, dev);
+    ZipitLCD *z = ZIPIT_LCD(dev);
     uint16_t val;
     if (z->selected) {
         z->buf[z->pos] = value & 0xff;
@@ -153,7 +156,7 @@ static void z2_lcd_cs(void *opaque, int line, int level)
 
 static void zipit_lcd_realize(SSISlave *dev, Error **errp)
 {
-    ZipitLCD *z = FROM_SSI_SLAVE(ZipitLCD, dev);
+    ZipitLCD *z = ZIPIT_LCD(dev);
     z->selected = 0;
     z->enabled = 0;
     z->pos = 0;
@@ -185,7 +188,7 @@ static void zipit_lcd_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo zipit_lcd_info = {
-    .name          = "zipit-lcd",
+    .name          = TYPE_ZIPIT_LCD,
     .parent        = TYPE_SSI_SLAVE,
     .instance_size = sizeof(ZipitLCD),
     .class_init    = zipit_lcd_class_init,
@@ -325,7 +328,7 @@ static void z2_init(MachineState *machine)
 
     type_register_static(&zipit_lcd_info);
     type_register_static(&aer915_info);
-    z2_lcd = ssi_create_slave(mpu->ssp[1], "zipit-lcd");
+    z2_lcd = ssi_create_slave(mpu->ssp[1], TYPE_ZIPIT_LCD);
     bus = pxa2xx_i2c_bus(mpu->i2c[0]);
     i2c_create_slave(bus, TYPE_AER915, 0x55);
     wm = i2c_create_slave(bus, TYPE_WM8750, 0x1b);
diff --git a/hw/display/ads7846.c b/hw/display/ads7846.c
index 9228b40b1af..56bf82fe079 100644
--- a/hw/display/ads7846.c
+++ b/hw/display/ads7846.c
@@ -29,6 +29,9 @@ typedef struct {
     int output;
 } ADS7846State;
 
+#define TYPE_ADS7846 "ads7846"
+#define ADS7846(obj) OBJECT_CHECK(ADS7846State, (obj), TYPE_ADS7846)
+
 /* Control-byte bitfields */
 #define CB_PD0		(1 << 0)
 #define CB_PD1		(1 << 1)
@@ -61,7 +64,7 @@ static void ads7846_int_update(ADS7846State *s)
 
 static uint32_t ads7846_transfer(SSISlave *dev, uint32_t value)
 {
-    ADS7846State *s = FROM_SSI_SLAVE(ADS7846State, dev);
+    ADS7846State *s = ADS7846(dev);
 
     switch (s->cycle ++) {
     case 0:
@@ -139,7 +142,7 @@ static const VMStateDescription vmstate_ads7846 = {
 static void ads7846_realize(SSISlave *d, Error **errp)
 {
     DeviceState *dev = DEVICE(d);
-    ADS7846State *s = FROM_SSI_SLAVE(ADS7846State, d);
+    ADS7846State *s = ADS7846(d);
 
     qdev_init_gpio_out(dev, &s->interrupt, 1);
 
@@ -166,7 +169,7 @@ static void ads7846_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo ads7846_info = {
-    .name          = "ads7846",
+    .name          = TYPE_ADS7846,
     .parent        = TYPE_SSI_SLAVE,
     .instance_size = sizeof(ADS7846State),
     .class_init    = ads7846_class_init,
diff --git a/hw/display/ssd0323.c b/hw/display/ssd0323.c
index c3bdb18742c..32d27f008ae 100644
--- a/hw/display/ssd0323.c
+++ b/hw/display/ssd0323.c
@@ -66,9 +66,13 @@ typedef struct {
     uint8_t framebuffer[128 * 80 / 2];
 } ssd0323_state;
 
+#define TYPE_SSD0323 "ssd0323"
+#define SSD0323(obj) OBJECT_CHECK(ssd0323_state, (obj), TYPE_SSD0323)
+
+
 static uint32_t ssd0323_transfer(SSISlave *dev, uint32_t data)
 {
-    ssd0323_state *s = FROM_SSI_SLAVE(ssd0323_state, dev);
+    ssd0323_state *s = SSD0323(dev);
 
     switch (s->mode) {
     case SSD0323_DATA:
@@ -346,7 +350,7 @@ static const GraphicHwOps ssd0323_ops = {
 static void ssd0323_realize(SSISlave *d, Error **errp)
 {
     DeviceState *dev = DEVICE(d);
-    ssd0323_state *s = FROM_SSI_SLAVE(ssd0323_state, d);
+    ssd0323_state *s = SSD0323(d);
 
     s->col_end = 63;
     s->row_end = 79;
@@ -368,7 +372,7 @@ static void ssd0323_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo ssd0323_info = {
-    .name          = "ssd0323",
+    .name          = TYPE_SSD0323,
     .parent        = TYPE_SSI_SLAVE,
     .instance_size = sizeof(ssd0323_state),
     .class_init    = ssd0323_class_init,
diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index 25cec2ddeaa..25cdf4c966d 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -74,7 +74,7 @@ typedef struct {
 
 static uint32_t ssi_sd_transfer(SSISlave *dev, uint32_t val)
 {
-    ssi_sd_state *s = FROM_SSI_SLAVE(ssi_sd_state, dev);
+    ssi_sd_state *s = SSI_SD(dev);
 
     /* Special case: allow CMD12 (STOP TRANSMISSION) while reading data.  */
     if (s->mode == SSI_SD_DATA_READ && val == 0x4d) {
@@ -241,7 +241,7 @@ static const VMStateDescription vmstate_ssi_sd = {
 
 static void ssi_sd_realize(SSISlave *d, Error **errp)
 {
-    ssi_sd_state *s = FROM_SSI_SLAVE(ssi_sd_state, d);
+    ssi_sd_state *s = SSI_SD(d);
     DeviceState *carddev;
     DriveInfo *dinfo;
     Error *err = NULL;
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PULL 34/34] Deprecate TileGX port
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (32 preceding siblings ...)
  2020-07-03 16:54 ` [PULL 33/34] Replace uses of FROM_SSI_SLAVE() macro with QOM casts Peter Maydell
@ 2020-07-03 16:54 ` Peter Maydell
  2020-07-03 17:50 ` [PULL 00/34] target-arm queue no-reply
  2020-07-04 17:43 ` Peter Maydell
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-03 16:54 UTC (permalink / raw)
  To: qemu-devel

Deprecate our TileGX target support:
 * we have no active maintainer for it
 * it has had essentially no contributions (other than tree-wide cleanups
   and similar) since it was first added
 * the Linux kernel dropped support in 2018, as has glibc

Note the deprecation in the manual, but don't try to print a warning
when QEMU runs -- printing unsuppressable messages is more obtrusive
for linux-user mode than it would be for system-emulation mode, and
it doesn't seem worth trying to invent a new suppressible-error
system for linux-user just for this.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-id: 20200619154831.26319-1-peter.maydell@linaro.org
---
 docs/system/deprecated.rst | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 843ae71fc61..47f84be8e09 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -404,6 +404,17 @@ The above, converted to the current supported format::
 
   json:{"file.driver":"rbd", "file.pool":"rbd", "file.image":"name"}
 
+linux-user mode CPUs
+--------------------
+
+``tilegx`` CPUs (since 5.1.0)
+'''''''''''''''''''''''''''''
+
+The ``tilegx`` guest CPU support (which was only implemented in
+linux-user mode) is deprecated and will be removed in a future version
+of QEMU. Support for this CPU was removed from the upstream Linux
+kernel in 2018, and has also been dropped from glibc.
+
 Related binaries
 ----------------
 
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* Re: [PULL 00/34] target-arm queue
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (33 preceding siblings ...)
  2020-07-03 16:54 ` [PULL 34/34] Deprecate TileGX port Peter Maydell
@ 2020-07-03 17:50 ` no-reply
  2020-07-04 17:43 ` Peter Maydell
  35 siblings, 0 replies; 44+ messages in thread
From: no-reply @ 2020-07-03 17:50 UTC (permalink / raw)
  To: peter.maydell; +Cc: qemu-devel

Patchew URL: https://patchew.org/QEMU/20200703165405.17672-1-peter.maydell@linaro.org/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PULL 00/34] target-arm queue
Type: series
Message-id: 20200703165405.17672-1-peter.maydell@linaro.org

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20200703165405.17672-1-peter.maydell@linaro.org -> patchew/20200703165405.17672-1-peter.maydell@linaro.org
Switched to a new branch 'test'
d638580 Deprecate TileGX port
1e2e778 Replace uses of FROM_SSI_SLAVE() macro with QOM casts
92a5f92 hw/arm/spitz: Provide usual QOM macros for corgi-ssp and spitz-lcdtg
d578f7a hw/arm/pxa2xx_pic: Use LOG_GUEST_ERROR for bad guest register accesses
8ca1ff9 hw/arm/spitz: Use LOG_GUEST_ERROR for bad guest register accesses
bdc8b3f hw/gpio/zaurus.c: Use LOG_GUEST_ERROR for bad guest register accesses
a7f0ce8 hw/arm/spitz: Encapsulate misc GPIO handling in a device
2d29801 hw/misc/max111x: Create header file for documentation, TYPE_ macros
1b20083 hw/misc/max111x: Use GPIO lines rather than max111x_set_input()
9ddef16 hw/arm/spitz: Use max111x properties to set initial values
208445b ssi: Add ssi_realize_and_unref()
7017444 hw/misc/max111x: Don't use vmstate_register()
57e564b hw/misc/max111x: provide QOM properties for setting initial values
4902bce hw/arm/spitz: Implement inbound GPIO lines for bit5 and power signals
d769576 hw/arm/spitz: Keep pointers to scp0, scp1 in SpitzMachineState
9970786 hw/arm/spitz: Keep pointers to MPU and SSI devices in SpitzMachineState
8943225 hw/arm/spitz: Create SpitzMachineClass abstract base class
2d5eafe hw/arm/spitz: Detabify
1f8dfdb hw/display/bcm2835_fb.c: Initialize all fields of struct
20ee56a target/arm: Fix temp double-free in sve ldr/str
705ce73 tests/acpi: virt: update golden masters for DSDT
c3b5af7 hw/arm/virt-acpi-build: Only expose flash on older machine types
e26cb37 tests/acpi: virt: allow DSDT acpi table changes
c824523 tests/acpi: remove stale allowed tables
4bcd8be target/arm: kvm: Handle misconfigured dabt injection
4dba079 target/arm: kvm: Handle DABT with no valid ISS
71f7055 hw/arm/virt: Let the virtio-iommu bypass MSIs
b692f4b virtio-iommu-pci: Add array of Interval properties
bf5c2b0 virtio-iommu: Handle reserved regions in the translation process
da96f35 virtio-iommu: Implement RESV_MEM probe request
b518252 qdev: Introduce DEFINE_PROP_RESERVED_REGION
88ac8a1 Select MDIO device 2 and 1 as PHY devices for i.MX6UL EVK board.
ab25de9 Add the ability to select a different PHY for each i.MX6UL FEC interface
0e700cc Add a phy-num property to the i.MX FEC emulator

=== OUTPUT BEGIN ===
1/34 Checking commit 0e700cc2f90d (Add a phy-num property to the i.MX FEC emulator)
2/34 Checking commit ab25de9b1194 (Add the ability to select a different PHY for each i.MX6UL FEC interface)
3/34 Checking commit 88ac8a1e6c55 (Select MDIO device 2 and 1 as PHY devices for i.MX6UL EVK board.)
4/34 Checking commit b518252f4b6a (qdev: Introduce DEFINE_PROP_RESERVED_REGION)
5/34 Checking commit da96f35271a3 (virtio-iommu: Implement RESV_MEM probe request)
6/34 Checking commit bf5c2b0b1ae2 (virtio-iommu: Handle reserved regions in the translation process)
7/34 Checking commit b692f4b0e005 (virtio-iommu-pci: Add array of Interval properties)
8/34 Checking commit 71f7055c32e1 (hw/arm/virt: Let the virtio-iommu bypass MSIs)
9/34 Checking commit 4dba079f1e38 (target/arm: kvm: Handle DABT with no valid ISS)
10/34 Checking commit 4bcd8bef2ef5 (target/arm: kvm: Handle misconfigured dabt injection)
11/34 Checking commit c824523e1b1f (tests/acpi: remove stale allowed tables)
12/34 Checking commit e26cb3737b8e (tests/acpi: virt: allow DSDT acpi table changes)
13/34 Checking commit c3b5af781c16 (hw/arm/virt-acpi-build: Only expose flash on older machine types)
14/34 Checking commit 705ce7328c86 (tests/acpi: virt: update golden masters for DSDT)
15/34 Checking commit 20ee56a97cef (target/arm: Fix temp double-free in sve ldr/str)
16/34 Checking commit 1f8dfdbe0e2f (hw/display/bcm2835_fb.c: Initialize all fields of struct)
17/34 Checking commit 2d5eafee3dfb (hw/arm/spitz: Detabify)
ERROR: space prohibited before that '++' (ctx:WxB)
#113: FILE: hw/arm/spitz.c:303:
+#define QUEUE_KEY(c)    s->fifo[(s->fifopos + s->fifolen ++) & 0xf] = c
                                                          ^

ERROR: Macros with complex values should be enclosed in parenthesis
#113: FILE: hw/arm/spitz.c:303:
+#define QUEUE_KEY(c)    s->fifo[(s->fifopos + s->fifolen ++) & 0xf] = c

total: 2 errors, 0 warnings, 259 lines checked

Patch 17/34 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

18/34 Checking commit 89432250560f (hw/arm/spitz: Create SpitzMachineClass abstract base class)
19/34 Checking commit 9970786d97e1 (hw/arm/spitz: Keep pointers to MPU and SSI devices in SpitzMachineState)
20/34 Checking commit d769576c9302 (hw/arm/spitz: Keep pointers to scp0, scp1 in SpitzMachineState)
21/34 Checking commit 4902bce57643 (hw/arm/spitz: Implement inbound GPIO lines for bit5 and power signals)
WARNING: line over 80 characters
#96: FILE: hw/arm/spitz.c:859:
+                              qdev_get_gpio_in_named(sms->lcdtg, "bl_power", 0));

total: 0 errors, 1 warnings, 68 lines checked

Patch 21/34 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
22/34 Checking commit 57e564bca41b (hw/misc/max111x: provide QOM properties for setting initial values)
23/34 Checking commit 701744470cb1 (hw/misc/max111x: Don't use vmstate_register())
24/34 Checking commit 208445bd32c9 (ssi: Add ssi_realize_and_unref())
25/34 Checking commit 9ddef165a4eb (hw/arm/spitz: Use max111x properties to set initial values)
WARNING: Block comments use a leading /* on a separate line
#31: FILE: hw/arm/spitz.c:736:
+    qdev_prop_set_uint8(sms->max1111, "input1" /* BATT_VOLT */,

WARNING: Block comments use a leading /* on a separate line
#33: FILE: hw/arm/spitz.c:738:
+    qdev_prop_set_uint8(sms->max1111, "input2" /* BATT_TEMP */, 0);

WARNING: Block comments use a leading /* on a separate line
#34: FILE: hw/arm/spitz.c:739:
+    qdev_prop_set_uint8(sms->max1111, "input3" /* ACIN_VOLT */,

total: 0 errors, 3 warnings, 18 lines checked

Patch 25/34 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
26/34 Checking commit 1b20083f7207 (hw/misc/max111x: Use GPIO lines rather than max111x_set_input())
27/34 Checking commit 2d29801356e9 (hw/misc/max111x: Create header file for documentation, TYPE_ macros)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#96: 
new file mode 100644

total: 0 errors, 1 warnings, 113 lines checked

Patch 27/34 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
28/34 Checking commit a7f0ce85ac48 (hw/arm/spitz: Encapsulate misc GPIO handling in a device)
WARNING: line over 80 characters
#186: FILE: hw/arm/spitz.c:893:
+                                qdev_get_gpio_in(sms->max1111, MAX1111_BATT_TEMP));

total: 0 errors, 1 warnings, 185 lines checked

Patch 28/34 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
29/34 Checking commit bdc8b3f67a7c (hw/gpio/zaurus.c: Use LOG_GUEST_ERROR for bad guest register accesses)
30/34 Checking commit 8ca1ff9ccf5b (hw/arm/spitz: Use LOG_GUEST_ERROR for bad guest register accesses)
31/34 Checking commit d578f7a7793b (hw/arm/pxa2xx_pic: Use LOG_GUEST_ERROR for bad guest register accesses)
32/34 Checking commit 92a5f929696f (hw/arm/spitz: Provide usual QOM macros for corgi-ssp and spitz-lcdtg)
33/34 Checking commit 1e2e778ea0bb (Replace uses of FROM_SSI_SLAVE() macro with QOM casts)
34/34 Checking commit d638580b6ac3 (Deprecate TileGX port)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200703165405.17672-1-peter.maydell@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PULL 00/34] target-arm queue
  2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
                   ` (34 preceding siblings ...)
  2020-07-03 17:50 ` [PULL 00/34] target-arm queue no-reply
@ 2020-07-04 17:43 ` Peter Maydell
  35 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2020-07-04 17:43 UTC (permalink / raw)
  To: QEMU Developers

On Fri, 3 Jul 2020 at 17:54, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> I might squeeze in another pullreq before softfreeze, but the
> queue was already big enough that I wanted to send this lot out now.
>
> -- PMM
>
> The following changes since commit 4abf70a661a5df3886ac9d7c19c3617fa92b922a:
>
>   Merge remote-tracking branch 'remotes/maxreitz/tags/pull-block-2020-06-24' into staging (2020-07-03 15:34:45 +0100)
>
> are available in the Git repository at:
>
>   https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20200703
>
> for you to fetch changes up to 0f10bf84a9d489259a5b11c6aa1b05c1175b76ea:
>
>   Deprecate TileGX port (2020-07-03 16:59:46 +0100)
>
> ----------------------------------------------------------------
> target-arm queue:
>  * i.MX6UL EVK board: put PHYs in the correct places
>  * hw/arm/virt: Let the virtio-iommu bypass MSIs
>  * target/arm: kvm: Handle DABT with no valid ISS
>  * hw/arm/virt-acpi-build: Only expose flash on older machine types
>  * target/arm: Fix temp double-free in sve ldr/str
>  * hw/display/bcm2835_fb.c: Initialize all fields of struct
>  * hw/arm/spitz: Code cleanup to fix Coverity-detected memory leak
>  * Deprecate TileGX port


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1
for any user-visible changes.

-- PMM


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PULL 05/34] virtio-iommu: Implement RESV_MEM probe request
  2020-07-03 16:53 ` [PULL 05/34] virtio-iommu: Implement RESV_MEM probe request Peter Maydell
@ 2020-07-05 18:21   ` Peter Maydell
  2020-07-08 14:40     ` Auger Eric
  0 siblings, 1 reply; 44+ messages in thread
From: Peter Maydell @ 2020-07-05 18:21 UTC (permalink / raw)
  To: QEMU Developers, Eric Auger

On Fri, 3 Jul 2020 at 17:54, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> From: Eric Auger <eric.auger@redhat.com>
>
> This patch implements the PROBE request. At the moment,
> only THE RESV_MEM property is handled. The first goal is
> to report iommu wide reserved regions such as the MSI regions
> set by the machine code. On x86 this will be the IOAPIC MSI
> region, [0xFEE00000 - 0xFEEFFFFF], on ARM this may be the ITS
> doorbell.

> @@ -452,6 +524,17 @@ static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
>          case VIRTIO_IOMMU_T_UNMAP:
>              tail.status = virtio_iommu_handle_unmap(s, iov, iov_cnt);
>              break;
> +        case VIRTIO_IOMMU_T_PROBE:
> +        {
> +            struct virtio_iommu_req_tail *ptail;
> +
> +            output_size = s->config.probe_size + sizeof(tail);
> +            buf = g_malloc0(output_size);
> +
> +            ptail = (struct virtio_iommu_req_tail *)
> +                        (buf + s->config.probe_size);
> +            ptail->status = virtio_iommu_handle_probe(s, iov, iov_cnt, buf);
> +        }
>          default:
>              tail.status = VIRTIO_IOMMU_S_UNSUPP;
>          }

Hi Eric -- Coverity points out (CID 1430180) that this new case
has neither a 'break' nor a /* fallthrough */ comment. Which is correct?

thanks
-- PMM


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PULL 05/34] virtio-iommu: Implement RESV_MEM probe request
  2020-07-05 18:21   ` Peter Maydell
@ 2020-07-08 14:40     ` Auger Eric
  0 siblings, 0 replies; 44+ messages in thread
From: Auger Eric @ 2020-07-08 14:40 UTC (permalink / raw)
  To: Peter Maydell, QEMU Developers

Hi Peter,

On 7/5/20 8:21 PM, Peter Maydell wrote:
> On Fri, 3 Jul 2020 at 17:54, Peter Maydell <peter.maydell@linaro.org> wrote:
>>
>> From: Eric Auger <eric.auger@redhat.com>
>>
>> This patch implements the PROBE request. At the moment,
>> only THE RESV_MEM property is handled. The first goal is
>> to report iommu wide reserved regions such as the MSI regions
>> set by the machine code. On x86 this will be the IOAPIC MSI
>> region, [0xFEE00000 - 0xFEEFFFFF], on ARM this may be the ITS
>> doorbell.
> 
>> @@ -452,6 +524,17 @@ static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
>>          case VIRTIO_IOMMU_T_UNMAP:
>>              tail.status = virtio_iommu_handle_unmap(s, iov, iov_cnt);
>>              break;
>> +        case VIRTIO_IOMMU_T_PROBE:
>> +        {
>> +            struct virtio_iommu_req_tail *ptail;
>> +
>> +            output_size = s->config.probe_size + sizeof(tail);
>> +            buf = g_malloc0(output_size);
>> +
>> +            ptail = (struct virtio_iommu_req_tail *)
>> +                        (buf + s->config.probe_size);
>> +            ptail->status = virtio_iommu_handle_probe(s, iov, iov_cnt, buf);
>> +        }
>>          default:
>>              tail.status = VIRTIO_IOMMU_S_UNSUPP;
>>          }
> 
> Hi Eric -- Coverity points out (CID 1430180) that this new case
> has neither a 'break' nor a /* fallthrough */ comment. Which is correct?
Thank you for reporting. Sure I will send a fix. This is harmless from a
functional pov.

Best Regards

Eric
> 
> thanks
> -- PMM
> 



^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PULL 08/34] hw/arm/virt: Let the virtio-iommu bypass MSIs
  2020-07-03 16:53 ` [PULL 08/34] hw/arm/virt: Let the virtio-iommu bypass MSIs Peter Maydell
@ 2023-02-02 10:47   ` Philippe Mathieu-Daudé
  2023-02-02 10:52     ` Philippe Mathieu-Daudé
  2023-02-02 10:58     ` Peter Maydell
  0 siblings, 2 replies; 44+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-02 10:47 UTC (permalink / raw)
  To: Eric Auger; +Cc: qemu-devel, Peter Maydell

Hi Eric,

On 3/7/20 17:53, Peter Maydell wrote:
> From: Eric Auger <eric.auger@redhat.com>
> 
> At the moment the virtio-iommu translates MSI transactions.
> This behavior is inherited from ARM SMMU. The virt machine
> code knows where the guest MSI doorbells are so we can easily
> declare those regions as VIRTIO_IOMMU_RESV_MEM_T_MSI. With that
> setting the guest will not map MSIs through the IOMMU and those
> transactions will be simply bypassed.
> 
> Depending on which MSI controller is in use (ITS or GICV2M),
> we declare either:
> - the ITS interrupt translation space (ITS_base + 0x10000),
>    containing the GITS_TRANSLATOR or
> - The GICV2M single frame, containing the MSI_SETSP_NS register.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Message-id: 20200629070404.10969-6-eric.auger@redhat.com
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   include/hw/arm/virt.h |  7 +++++++
>   hw/arm/virt.c         | 30 ++++++++++++++++++++++++++++++
>   2 files changed, 37 insertions(+)


>   static void create_gic(VirtMachineState *vms)
> @@ -2198,8 +2200,36 @@ out:
>   static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>                                               DeviceState *dev, Error **errp)
>   {
> +    VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
> +
>       if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
>           virt_memory_pre_plug(hotplug_dev, dev, errp);
> +    } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
> +        hwaddr db_start = 0, db_end = 0;
> +        char *resv_prop_str;
> +
> +        switch (vms->msi_controller) {
> +        case VIRT_MSI_CTRL_NONE:
> +            return;
> +        case VIRT_MSI_CTRL_ITS:
> +            /* GITS_TRANSLATER page */
> +            db_start = base_memmap[VIRT_GIC_ITS].base + 0x10000;
> +            db_end = base_memmap[VIRT_GIC_ITS].base +
> +                     base_memmap[VIRT_GIC_ITS].size - 1;
> +            break;
> +        case VIRT_MSI_CTRL_GICV2M:
> +            /* MSI_SETSPI_NS page */
> +            db_start = base_memmap[VIRT_GIC_V2M].base;
> +            db_end = db_start + base_memmap[VIRT_GIC_V2M].size - 1;
> +            break;
> +        }
> +        resv_prop_str = g_strdup_printf("0x%"PRIx64":0x%"PRIx64":%u",
> +                                        db_start, db_end,
> +                                        VIRTIO_IOMMU_RESV_MEM_T_MSI);
> +
> +        qdev_prop_set_uint32(dev, "len-reserved-regions", 1);

Where is "len-reserved-regions" declared?

Since qdev_prop_set_uint32() uses &error_abort, isn't this call
aborting the process? I am confused how this code path is exercised,
what am I missing?

> +        qdev_prop_set_string(dev, "reserved-regions[0]", resv_prop_str);
> +        g_free(resv_prop_str);
>       }
>   }
>   


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PULL 08/34] hw/arm/virt: Let the virtio-iommu bypass MSIs
  2023-02-02 10:47   ` Philippe Mathieu-Daudé
@ 2023-02-02 10:52     ` Philippe Mathieu-Daudé
  2023-02-02 10:58     ` Peter Maydell
  1 sibling, 0 replies; 44+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-02 10:52 UTC (permalink / raw)
  To: Eric Auger; +Cc: qemu-devel, Peter Maydell

On 2/2/23 11:47, Philippe Mathieu-Daudé wrote:
> Hi Eric,
> 
> On 3/7/20 17:53, Peter Maydell wrote:
>> From: Eric Auger <eric.auger@redhat.com>
>>
>> At the moment the virtio-iommu translates MSI transactions.
>> This behavior is inherited from ARM SMMU. The virt machine
>> code knows where the guest MSI doorbells are so we can easily
>> declare those regions as VIRTIO_IOMMU_RESV_MEM_T_MSI. With that
>> setting the guest will not map MSIs through the IOMMU and those
>> transactions will be simply bypassed.
>>
>> Depending on which MSI controller is in use (ITS or GICV2M),
>> we declare either:
>> - the ITS interrupt translation space (ITS_base + 0x10000),
>>    containing the GITS_TRANSLATOR or
>> - The GICV2M single frame, containing the MSI_SETSP_NS register.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> Message-id: 20200629070404.10969-6-eric.auger@redhat.com
>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>   include/hw/arm/virt.h |  7 +++++++
>>   hw/arm/virt.c         | 30 ++++++++++++++++++++++++++++++
>>   2 files changed, 37 insertions(+)
> 
> 
>>   static void create_gic(VirtMachineState *vms)
>> @@ -2198,8 +2200,36 @@ out:
>>   static void virt_machine_device_pre_plug_cb(HotplugHandler 
>> *hotplug_dev,
>>                                               DeviceState *dev, Error 
>> **errp)
>>   {
>> +    VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
>> +
>>       if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
>>           virt_memory_pre_plug(hotplug_dev, dev, errp);
>> +    } else if (object_dynamic_cast(OBJECT(dev), 
>> TYPE_VIRTIO_IOMMU_PCI)) {
>> +        hwaddr db_start = 0, db_end = 0;
>> +        char *resv_prop_str;
>> +
>> +        switch (vms->msi_controller) {
>> +        case VIRT_MSI_CTRL_NONE:
>> +            return;
>> +        case VIRT_MSI_CTRL_ITS:
>> +            /* GITS_TRANSLATER page */
>> +            db_start = base_memmap[VIRT_GIC_ITS].base + 0x10000;
>> +            db_end = base_memmap[VIRT_GIC_ITS].base +
>> +                     base_memmap[VIRT_GIC_ITS].size - 1;
>> +            break;
>> +        case VIRT_MSI_CTRL_GICV2M:
>> +            /* MSI_SETSPI_NS page */
>> +            db_start = base_memmap[VIRT_GIC_V2M].base;
>> +            db_end = db_start + base_memmap[VIRT_GIC_V2M].size - 1;
>> +            break;
>> +        }
>> +        resv_prop_str = g_strdup_printf("0x%"PRIx64":0x%"PRIx64":%u",
>> +                                        db_start, db_end,
>> +                                        VIRTIO_IOMMU_RESV_MEM_T_MSI);
>> +
>> +        qdev_prop_set_uint32(dev, "len-reserved-regions", 1);
> 
> Where is "len-reserved-regions" declared?
> 
> Since qdev_prop_set_uint32() uses &error_abort, isn't this call
> aborting the process? I am confused how this code path is exercised,
> what am I missing?

The call path is:

   qdev_prop_set_uint32 ->
     object_property_set_int ->
       object_property_set_qobject ->
         object_property_set ->
           object_property_find_err

So QEMU should abort displaying:

"Property 'virtio-iommu-pci.len-reserved-regions' not found".

>> +        qdev_prop_set_string(dev, "reserved-regions[0]", resv_prop_str);
>> +        g_free(resv_prop_str);
>>       }
>>   }



^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PULL 08/34] hw/arm/virt: Let the virtio-iommu bypass MSIs
  2023-02-02 10:47   ` Philippe Mathieu-Daudé
  2023-02-02 10:52     ` Philippe Mathieu-Daudé
@ 2023-02-02 10:58     ` Peter Maydell
  2023-02-02 11:07       ` Philippe Mathieu-Daudé
  2023-02-02 13:06       ` Eric Auger
  1 sibling, 2 replies; 44+ messages in thread
From: Peter Maydell @ 2023-02-02 10:58 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: Eric Auger, qemu-devel

On Thu, 2 Feb 2023 at 10:47, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> Where is "len-reserved-regions" declared?

  DEFINE_PROP_ARRAY("reserved-regions", ...)

does this. For an array property "foo" the machinery creates an integer
property "foo-len", which must be set first. Setting that
then creates properties "foo[0]", "foo[1]", ... which can be set.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PULL 08/34] hw/arm/virt: Let the virtio-iommu bypass MSIs
  2023-02-02 10:58     ` Peter Maydell
@ 2023-02-02 11:07       ` Philippe Mathieu-Daudé
  2023-02-02 13:06       ` Eric Auger
  1 sibling, 0 replies; 44+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-02 11:07 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Eric Auger, qemu-devel

On 2/2/23 11:58, Peter Maydell wrote:
> On Thu, 2 Feb 2023 at 10:47, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>> Where is "len-reserved-regions" declared?
> 
>    DEFINE_PROP_ARRAY("reserved-regions", ...)
> 
> does this. For an array property "foo" the machinery creates an integer
> property "foo-len", which must be set first. Setting that
> then creates properties "foo[0]", "foo[1]", ... which can be set.

Oh indeed now I see the array prefix:

   #define PROP_ARRAY_LEN_PREFIX "len-"

Not obvious to realize while grepping.

Thanks for the quick answer!



^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PULL 08/34] hw/arm/virt: Let the virtio-iommu bypass MSIs
  2023-02-02 10:58     ` Peter Maydell
  2023-02-02 11:07       ` Philippe Mathieu-Daudé
@ 2023-02-02 13:06       ` Eric Auger
  1 sibling, 0 replies; 44+ messages in thread
From: Eric Auger @ 2023-02-02 13:06 UTC (permalink / raw)
  To: Peter Maydell, Philippe Mathieu-Daudé; +Cc: qemu-devel

Hi,
On 2/2/23 11:58, Peter Maydell wrote:
> On Thu, 2 Feb 2023 at 10:47, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>> Where is "len-reserved-regions" declared?
>   DEFINE_PROP_ARRAY("reserved-regions", ...)
>
> does this. For an array property "foo" the machinery creates an integer
> property "foo-len", which must be set first. Setting that
> then creates properties "foo[0]", "foo[1]", ... which can be set.

Yes. Thank you Peter!

Eric
>
> thanks
> -- PMM
>



^ permalink raw reply	[flat|nested] 44+ messages in thread

end of thread, other threads:[~2023-02-02 13:06 UTC | newest]

Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-03 16:53 [PULL 00/34] target-arm queue Peter Maydell
2020-07-03 16:53 ` [PULL 01/34] Add a phy-num property to the i.MX FEC emulator Peter Maydell
2020-07-03 16:53 ` [PULL 02/34] Add the ability to select a different PHY for each i.MX6UL FEC interface Peter Maydell
2020-07-03 16:53 ` [PULL 03/34] Select MDIO device 2 and 1 as PHY devices for i.MX6UL EVK board Peter Maydell
2020-07-03 16:53 ` [PULL 04/34] qdev: Introduce DEFINE_PROP_RESERVED_REGION Peter Maydell
2020-07-03 16:53 ` [PULL 05/34] virtio-iommu: Implement RESV_MEM probe request Peter Maydell
2020-07-05 18:21   ` Peter Maydell
2020-07-08 14:40     ` Auger Eric
2020-07-03 16:53 ` [PULL 06/34] virtio-iommu: Handle reserved regions in the translation process Peter Maydell
2020-07-03 16:53 ` [PULL 07/34] virtio-iommu-pci: Add array of Interval properties Peter Maydell
2020-07-03 16:53 ` [PULL 08/34] hw/arm/virt: Let the virtio-iommu bypass MSIs Peter Maydell
2023-02-02 10:47   ` Philippe Mathieu-Daudé
2023-02-02 10:52     ` Philippe Mathieu-Daudé
2023-02-02 10:58     ` Peter Maydell
2023-02-02 11:07       ` Philippe Mathieu-Daudé
2023-02-02 13:06       ` Eric Auger
2020-07-03 16:53 ` [PULL 09/34] target/arm: kvm: Handle DABT with no valid ISS Peter Maydell
2020-07-03 16:53 ` [PULL 10/34] target/arm: kvm: Handle misconfigured dabt injection Peter Maydell
2020-07-03 16:53 ` [PULL 11/34] tests/acpi: remove stale allowed tables Peter Maydell
2020-07-03 16:53 ` [PULL 12/34] tests/acpi: virt: allow DSDT acpi table changes Peter Maydell
2020-07-03 16:53 ` [PULL 13/34] hw/arm/virt-acpi-build: Only expose flash on older machine types Peter Maydell
2020-07-03 16:53 ` [PULL 14/34] tests/acpi: virt: update golden masters for DSDT Peter Maydell
2020-07-03 16:53 ` [PULL 15/34] target/arm: Fix temp double-free in sve ldr/str Peter Maydell
2020-07-03 16:53 ` [PULL 16/34] hw/display/bcm2835_fb.c: Initialize all fields of struct Peter Maydell
2020-07-03 16:53 ` [PULL 17/34] hw/arm/spitz: Detabify Peter Maydell
2020-07-03 16:53 ` [PULL 18/34] hw/arm/spitz: Create SpitzMachineClass abstract base class Peter Maydell
2020-07-03 16:53 ` [PULL 19/34] hw/arm/spitz: Keep pointers to MPU and SSI devices in SpitzMachineState Peter Maydell
2020-07-03 16:53 ` [PULL 20/34] hw/arm/spitz: Keep pointers to scp0, scp1 " Peter Maydell
2020-07-03 16:53 ` [PULL 21/34] hw/arm/spitz: Implement inbound GPIO lines for bit5 and power signals Peter Maydell
2020-07-03 16:53 ` [PULL 22/34] hw/misc/max111x: provide QOM properties for setting initial values Peter Maydell
2020-07-03 16:53 ` [PULL 23/34] hw/misc/max111x: Don't use vmstate_register() Peter Maydell
2020-07-03 16:53 ` [PULL 24/34] ssi: Add ssi_realize_and_unref() Peter Maydell
2020-07-03 16:53 ` [PULL 25/34] hw/arm/spitz: Use max111x properties to set initial values Peter Maydell
2020-07-03 16:53 ` [PULL 26/34] hw/misc/max111x: Use GPIO lines rather than max111x_set_input() Peter Maydell
2020-07-03 16:53 ` [PULL 27/34] hw/misc/max111x: Create header file for documentation, TYPE_ macros Peter Maydell
2020-07-03 16:53 ` [PULL 28/34] hw/arm/spitz: Encapsulate misc GPIO handling in a device Peter Maydell
2020-07-03 16:54 ` [PULL 29/34] hw/gpio/zaurus.c: Use LOG_GUEST_ERROR for bad guest register accesses Peter Maydell
2020-07-03 16:54 ` [PULL 30/34] hw/arm/spitz: " Peter Maydell
2020-07-03 16:54 ` [PULL 31/34] hw/arm/pxa2xx_pic: " Peter Maydell
2020-07-03 16:54 ` [PULL 32/34] hw/arm/spitz: Provide usual QOM macros for corgi-ssp and spitz-lcdtg Peter Maydell
2020-07-03 16:54 ` [PULL 33/34] Replace uses of FROM_SSI_SLAVE() macro with QOM casts Peter Maydell
2020-07-03 16:54 ` [PULL 34/34] Deprecate TileGX port Peter Maydell
2020-07-03 17:50 ` [PULL 00/34] target-arm queue no-reply
2020-07-04 17:43 ` Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.