* [PULL 1/4] hw/timer/slavio_timer: Allow 64-bit accesses
2021-01-06 12:05 [PULL 0/4] qemu-sparc queue 20210106 Mark Cave-Ayland
@ 2021-01-06 12:05 ` Mark Cave-Ayland
2021-01-06 12:05 ` [PULL 2/4] hw/sparc: Make grlib-irqmp device handle its own inbound IRQ lines Mark Cave-Ayland
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Mark Cave-Ayland @ 2021-01-06 12:05 UTC (permalink / raw)
To: peter.maydell, qemu-devel
Cc: Yap KV, Mark Cave-Ayland, Philippe Mathieu-Daudé,
qemu-stable
From: Philippe Mathieu-Daudé <f4bug@amsat.org>
Per the "NCR89C105 Chip Specification" referenced in the header:
Chip-level Address Map
------------------------------------------------------------------
| 1D0 0000 -> | Counter/Timers | W,D |
| 1DF FFFF | | |
...
The address map indicated the allowed accesses at each address.
[...] W indicates a word access, and D indicates a double-word
access.
The SLAVIO timer controller is implemented expecting 32-bit accesses.
Commit a3d12d073e1 restricted the memory accesses to 32-bit, while
the device allows 64-bit accesses.
This was not an issue until commit 5d971f9e67 which reverted
("memory: accept mismatching sizes in memory_region_access_valid").
Fix by renaming .valid MemoryRegionOps as .impl, and add the valid
access range (W -> 4, D -> 8).
Since commit 21786c7e598 ("memory: Log invalid memory accesses")
this class of bug can be quickly debugged displaying 'guest_errors'
accesses, as:
$ qemu-system-sparc -M SS-20 -m 256 -bios ss20_v2.25_rom -serial stdio -d guest_errors
Power-ON Reset
Invalid access at addr 0x0, size 8, region 'timer-1', reason: invalid size (min:4 max:4)
$ qemu-system-sparc -M SS-20 -m 256 -bios ss20_v2.25_rom -monitor stdio -S
(qemu) info mtree
address-space: memory
0000000000000000-ffffffffffffffff (prio 0, i/o): system
...
0000000ff1300000-0000000ff130000f (prio 0, i/o): timer-1
^^^^^^^^^ ^^^^^^^
\ memory region base address and name /
(qemu) info qtree
bus: main-system-bus
dev: slavio_timer, id "" <-- device type name
gpio-out "sysbus-irq" 17
num_cpus = 1 (0x1)
mmio 0000000ff1310000/0000000000000014
mmio 0000000ff1300000/0000000000000010 <--- base address
mmio 0000000ff1301000/0000000000000010
mmio 0000000ff1302000/0000000000000010
...
Reported-by: Yap KV <yapkv@yahoo.com>
Buglink: https://bugs.launchpad.net/bugs/1906905
Fixes: a3d12d073e1 ("slavio_timer: convert to memory API")
CC: qemu-stable@nongnu.org
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20201205150903.3062711-1-f4bug@amsat.org>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
hw/timer/slavio_timer.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/hw/timer/slavio_timer.c b/hw/timer/slavio_timer.c
index 5b2d20cb6a..03e33fc592 100644
--- a/hw/timer/slavio_timer.c
+++ b/hw/timer/slavio_timer.c
@@ -331,6 +331,10 @@ static const MemoryRegionOps slavio_timer_mem_ops = {
.write = slavio_timer_mem_writel,
.endianness = DEVICE_NATIVE_ENDIAN,
.valid = {
+ .min_access_size = 4,
+ .max_access_size = 8,
+ },
+ .impl = {
.min_access_size = 4,
.max_access_size = 4,
},
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PULL 2/4] hw/sparc: Make grlib-irqmp device handle its own inbound IRQ lines
2021-01-06 12:05 [PULL 0/4] qemu-sparc queue 20210106 Mark Cave-Ayland
2021-01-06 12:05 ` [PULL 1/4] hw/timer/slavio_timer: Allow 64-bit accesses Mark Cave-Ayland
@ 2021-01-06 12:05 ` Mark Cave-Ayland
2021-01-06 12:05 ` [PULL 3/4] include/hw/sparc/grlib.h: Remove unused set_pil_in_fn typedef Mark Cave-Ayland
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Mark Cave-Ayland @ 2021-01-06 12:05 UTC (permalink / raw)
To: peter.maydell, qemu-devel; +Cc: KONRAD Frederic, Mark Cave-Ayland
From: Peter Maydell <peter.maydell@linaro.org>
Currently the GRLIB_IRQMP device is used in one place (the leon3 board),
but instead of the device providing inbound gpio lines for the board
to wire up, the board code itself calls qemu_allocate_irqs() with
the handler function being a set_irq function defined in the code
for the device.
Refactor this into the standard setup of a device having input
gpio lines.
This fixes a trivial Coverity memory leak report (the leon3
board code leaks the IRQ array returned from qemu_allocate_irqs()).
Fixes: Coverity CID 1421922
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20201212144134.29594-2-peter.maydell@linaro.org>
Reviewed-by: KONRAD Frederic <frederic.konrad@adacore.com>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
hw/intc/grlib_irqmp.c | 5 ++++-
hw/sparc/leon3.c | 21 +++++++++------------
include/hw/sparc/grlib.h | 2 --
3 files changed, 13 insertions(+), 15 deletions(-)
diff --git a/hw/intc/grlib_irqmp.c b/hw/intc/grlib_irqmp.c
index ffec4a07ee..984334fa7b 100644
--- a/hw/intc/grlib_irqmp.c
+++ b/hw/intc/grlib_irqmp.c
@@ -51,6 +51,8 @@
#define FORCE_OFFSET 0x80
#define EXTENDED_OFFSET 0xC0
+#define MAX_PILS 16
+
OBJECT_DECLARE_SIMPLE_TYPE(IRQMP, GRLIB_IRQMP)
typedef struct IRQMPState IRQMPState;
@@ -126,7 +128,7 @@ void grlib_irqmp_ack(DeviceState *dev, int intno)
grlib_irqmp_ack_mask(state, mask);
}
-void grlib_irqmp_set_irq(void *opaque, int irq, int level)
+static void grlib_irqmp_set_irq(void *opaque, int irq, int level)
{
IRQMP *irqmp = GRLIB_IRQMP(opaque);
IRQMPState *s;
@@ -328,6 +330,7 @@ static void grlib_irqmp_init(Object *obj)
IRQMP *irqmp = GRLIB_IRQMP(obj);
SysBusDevice *dev = SYS_BUS_DEVICE(obj);
+ qdev_init_gpio_in(DEVICE(obj), grlib_irqmp_set_irq, MAX_PILS);
qdev_init_gpio_out_named(DEVICE(obj), &irqmp->irq, "grlib-irq", 1);
memory_region_init_io(&irqmp->iomem, obj, &grlib_irqmp_ops, irqmp,
"irqmp", IRQMP_REG_SIZE);
diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
index 4bc4ebea84..7e16eea9e6 100644
--- a/hw/sparc/leon3.c
+++ b/hw/sparc/leon3.c
@@ -52,8 +52,6 @@
#define LEON3_PROM_OFFSET (0x00000000)
#define LEON3_RAM_OFFSET (0x40000000)
-#define MAX_PILS 16
-
#define LEON3_UART_OFFSET (0x80000100)
#define LEON3_UART_IRQ (3)
@@ -194,11 +192,10 @@ static void leon3_generic_hw_init(MachineState *machine)
MemoryRegion *prom = g_new(MemoryRegion, 1);
int ret;
char *filename;
- qemu_irq *cpu_irqs = NULL;
int bios_size;
int prom_size;
ResetData *reset_info;
- DeviceState *dev;
+ DeviceState *dev, *irqmpdev;
int i;
AHBPnp *ahb_pnp;
APBPnp *apb_pnp;
@@ -230,16 +227,15 @@ static void leon3_generic_hw_init(MachineState *machine)
GRLIB_AHB_SLAVE, GRLIB_AHBMEM_AREA);
/* Allocate IRQ manager */
- dev = qdev_new(TYPE_GRLIB_IRQMP);
+ irqmpdev = qdev_new(TYPE_GRLIB_IRQMP);
qdev_init_gpio_in_named_with_opaque(DEVICE(cpu), leon3_set_pil_in,
env, "pil", 1);
- qdev_connect_gpio_out_named(dev, "grlib-irq", 0,
+ qdev_connect_gpio_out_named(irqmpdev, "grlib-irq", 0,
qdev_get_gpio_in_named(DEVICE(cpu), "pil", 0));
- sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
- sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, LEON3_IRQMP_OFFSET);
- env->irq_manager = dev;
+ sysbus_realize_and_unref(SYS_BUS_DEVICE(irqmpdev), &error_fatal);
+ sysbus_mmio_map(SYS_BUS_DEVICE(irqmpdev), 0, LEON3_IRQMP_OFFSET);
+ env->irq_manager = irqmpdev;
env->qemu_irq_ack = leon3_irq_manager;
- cpu_irqs = qemu_allocate_irqs(grlib_irqmp_set_irq, dev, MAX_PILS);
grlib_apb_pnp_add_entry(apb_pnp, LEON3_IRQMP_OFFSET, 0xFFF,
GRLIB_VENDOR_GAISLER, GRLIB_IRQMP_DEV,
2, 0, GRLIB_APBIO_AREA);
@@ -330,7 +326,7 @@ static void leon3_generic_hw_init(MachineState *machine)
sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, LEON3_TIMER_OFFSET);
for (i = 0; i < LEON3_TIMER_COUNT; i++) {
sysbus_connect_irq(SYS_BUS_DEVICE(dev), i,
- cpu_irqs[LEON3_TIMER_IRQ + i]);
+ qdev_get_gpio_in(irqmpdev, LEON3_TIMER_IRQ + i));
}
grlib_apb_pnp_add_entry(apb_pnp, LEON3_TIMER_OFFSET, 0xFFF,
@@ -342,7 +338,8 @@ static void leon3_generic_hw_init(MachineState *machine)
qdev_prop_set_chr(dev, "chrdev", serial_hd(0));
sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, LEON3_UART_OFFSET);
- sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, cpu_irqs[LEON3_UART_IRQ]);
+ sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0,
+ qdev_get_gpio_in(irqmpdev, LEON3_UART_IRQ));
grlib_apb_pnp_add_entry(apb_pnp, LEON3_UART_OFFSET, 0xFFF,
GRLIB_VENDOR_GAISLER, GRLIB_APBUART_DEV, 1,
LEON3_UART_IRQ, GRLIB_APBIO_AREA);
diff --git a/include/hw/sparc/grlib.h b/include/hw/sparc/grlib.h
index 78b6178fcd..e1d1beaa73 100644
--- a/include/hw/sparc/grlib.h
+++ b/include/hw/sparc/grlib.h
@@ -36,8 +36,6 @@
typedef void (*set_pil_in_fn) (void *opaque, uint32_t pil_in);
-void grlib_irqmp_set_irq(void *opaque, int irq, int level);
-
void grlib_irqmp_ack(DeviceState *dev, int intno);
/* GPTimer */
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PULL 3/4] include/hw/sparc/grlib.h: Remove unused set_pil_in_fn typedef
2021-01-06 12:05 [PULL 0/4] qemu-sparc queue 20210106 Mark Cave-Ayland
2021-01-06 12:05 ` [PULL 1/4] hw/timer/slavio_timer: Allow 64-bit accesses Mark Cave-Ayland
2021-01-06 12:05 ` [PULL 2/4] hw/sparc: Make grlib-irqmp device handle its own inbound IRQ lines Mark Cave-Ayland
@ 2021-01-06 12:05 ` Mark Cave-Ayland
2021-01-06 12:05 ` [PULL 4/4] sun4m: don't connect two qemu_irqs directly to the same input Mark Cave-Ayland
2021-01-06 15:51 ` [PULL 0/4] qemu-sparc queue 20210106 Peter Maydell
4 siblings, 0 replies; 6+ messages in thread
From: Mark Cave-Ayland @ 2021-01-06 12:05 UTC (permalink / raw)
To: peter.maydell, qemu-devel; +Cc: KONRAD Frederic, Mark Cave-Ayland
From: Peter Maydell <peter.maydell@linaro.org>
The grlib.h header defines a set_pil_in_fn typedef which is never
used; remove it.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20201212144134.29594-3-peter.maydell@linaro.org>
Reviewed-by: KONRAD Frederic <frederic.konrad@adacore.com>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
include/hw/sparc/grlib.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/include/hw/sparc/grlib.h b/include/hw/sparc/grlib.h
index e1d1beaa73..2104f493f3 100644
--- a/include/hw/sparc/grlib.h
+++ b/include/hw/sparc/grlib.h
@@ -34,8 +34,6 @@
/* IRQMP */
#define TYPE_GRLIB_IRQMP "grlib,irqmp"
-typedef void (*set_pil_in_fn) (void *opaque, uint32_t pil_in);
-
void grlib_irqmp_ack(DeviceState *dev, int intno);
/* GPTimer */
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PULL 4/4] sun4m: don't connect two qemu_irqs directly to the same input
2021-01-06 12:05 [PULL 0/4] qemu-sparc queue 20210106 Mark Cave-Ayland
` (2 preceding siblings ...)
2021-01-06 12:05 ` [PULL 3/4] include/hw/sparc/grlib.h: Remove unused set_pil_in_fn typedef Mark Cave-Ayland
@ 2021-01-06 12:05 ` Mark Cave-Ayland
2021-01-06 15:51 ` [PULL 0/4] qemu-sparc queue 20210106 Peter Maydell
4 siblings, 0 replies; 6+ messages in thread
From: Mark Cave-Ayland @ 2021-01-06 12:05 UTC (permalink / raw)
To: peter.maydell, qemu-devel; +Cc: Mark Cave-Ayland, Artyom Tarasenko
The sun4m board code connects both of the IRQ outputs of each ESCC to the
same slavio input qemu_irq. Connecting two qemu_irqs outputs directly to the
same input is not valid as it produces subtly wrong behaviour (for instance
if both the IRQ lines are high, and then one goes low, the PIC input will see
this as a high-to-low transition even though the second IRQ line should still
be holding it high).
This kind of wiring needs an explicitly created OR gate; add one.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Message-Id: <20201219111934.5540-1-mark.cave-ayland@ilande.co.uk>
Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
hw/sparc/Kconfig | 1 +
hw/sparc/sun4m.c | 24 +++++++++++++++++++-----
2 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/hw/sparc/Kconfig b/hw/sparc/Kconfig
index 91805afab6..8dcb10086f 100644
--- a/hw/sparc/Kconfig
+++ b/hw/sparc/Kconfig
@@ -14,6 +14,7 @@ config SUN4M
select M48T59
select STP2000
select CHRP_NVRAM
+ select OR_IRQ
config LEON3
bool
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 8686371318..38ca1e33c7 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -50,6 +50,7 @@
#include "hw/misc/empty_slot.h"
#include "hw/misc/unimp.h"
#include "hw/irq.h"
+#include "hw/or-irq.h"
#include "hw/loader.h"
#include "elf.h"
#include "trace.h"
@@ -848,7 +849,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
uint32_t initrd_size;
DriveInfo *fd[MAX_FD];
FWCfgState *fw_cfg;
- DeviceState *dev;
+ DeviceState *dev, *ms_kb_orgate, *serial_orgate;
SysBusDevice *s;
unsigned int smp_cpus = machine->smp.cpus;
unsigned int max_cpus = machine->smp.max_cpus;
@@ -994,10 +995,16 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
qdev_prop_set_uint32(dev, "chnAtype", escc_kbd);
s = SYS_BUS_DEVICE(dev);
sysbus_realize_and_unref(s, &error_fatal);
- sysbus_connect_irq(s, 0, slavio_irq[14]);
- sysbus_connect_irq(s, 1, slavio_irq[14]);
sysbus_mmio_map(s, 0, hwdef->ms_kb_base);
+ /* Logically OR both its IRQs together */
+ ms_kb_orgate = DEVICE(object_new(TYPE_OR_IRQ));
+ object_property_set_int(OBJECT(ms_kb_orgate), "num-lines", 2, &error_fatal);
+ qdev_realize_and_unref(ms_kb_orgate, NULL, &error_fatal);
+ sysbus_connect_irq(s, 0, qdev_get_gpio_in(ms_kb_orgate, 0));
+ sysbus_connect_irq(s, 1, qdev_get_gpio_in(ms_kb_orgate, 1));
+ qdev_connect_gpio_out(DEVICE(ms_kb_orgate), 0, slavio_irq[14]);
+
dev = qdev_new(TYPE_ESCC);
qdev_prop_set_uint32(dev, "disabled", 0);
qdev_prop_set_uint32(dev, "frequency", ESCC_CLOCK);
@@ -1009,10 +1016,17 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
s = SYS_BUS_DEVICE(dev);
sysbus_realize_and_unref(s, &error_fatal);
- sysbus_connect_irq(s, 0, slavio_irq[15]);
- sysbus_connect_irq(s, 1, slavio_irq[15]);
sysbus_mmio_map(s, 0, hwdef->serial_base);
+ /* Logically OR both its IRQs together */
+ serial_orgate = DEVICE(object_new(TYPE_OR_IRQ));
+ object_property_set_int(OBJECT(serial_orgate), "num-lines", 2,
+ &error_fatal);
+ qdev_realize_and_unref(serial_orgate, NULL, &error_fatal);
+ sysbus_connect_irq(s, 0, qdev_get_gpio_in(serial_orgate, 0));
+ sysbus_connect_irq(s, 1, qdev_get_gpio_in(serial_orgate, 1));
+ qdev_connect_gpio_out(DEVICE(serial_orgate), 0, slavio_irq[15]);
+
if (hwdef->apc_base) {
apc_init(hwdef->apc_base, qemu_allocate_irq(cpu_halt_signal, NULL, 0));
}
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PULL 0/4] qemu-sparc queue 20210106
2021-01-06 12:05 [PULL 0/4] qemu-sparc queue 20210106 Mark Cave-Ayland
` (3 preceding siblings ...)
2021-01-06 12:05 ` [PULL 4/4] sun4m: don't connect two qemu_irqs directly to the same input Mark Cave-Ayland
@ 2021-01-06 15:51 ` Peter Maydell
4 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2021-01-06 15:51 UTC (permalink / raw)
To: Mark Cave-Ayland; +Cc: QEMU Developers
On Wed, 6 Jan 2021 at 12:05, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> The following changes since commit 2e0b5bbe813930021b2baab03c9d424c1c52d18b:
>
> Merge remote-tracking branch 'remotes/philmd-gitlab/tags/mips-20210104' into staging (2021-01-05 21:06:42 +0000)
>
> are available in the Git repository at:
>
> git://github.com/mcayland/qemu.git tags/qemu-sparc-20210106
>
> for you to fetch changes up to a879306ca14de576d3a5dd51f830ebf89753e223:
>
> sun4m: don't connect two qemu_irqs directly to the same input (2021-01-06 11:41:37 +0000)
>
> ----------------------------------------------------------------
> qemu-sparc queue
>
> ----------------------------------------------------------------
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/6.0
for any user-visible changes.
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread