Re: [Qemu-devel] [PATCH] SiFive RISC-V GPIO Device
On Tue, 26 Mar 2019 10:49:11 PDT (-0700), chout...@adacore.com wrote: Hi Palmer, On 26/03/2019 09:58, Palmer Dabbelt wrote: Do you have anything that actually glues this to a machine so I can test it? In this patch I do instantiate the device in sifive_e machine. OK, that's what I thought but I couldn't figure out how to actually talk to the device. Hopefully it's just an issue on my end.
Re: [Qemu-devel] [PATCH] SiFive RISC-V GPIO Device
Hi Palmer, On 26/03/2019 09:58, Palmer Dabbelt wrote: > Do you have anything that actually glues this to a machine so I can test it? > In this patch I do instantiate the device in sifive_e machine. Regards,
Re: [Qemu-devel] [PATCH] SiFive RISC-V GPIO Device
On Tue, 12 Feb 2019 09:38:39 PST (-0800), chout...@adacore.com wrote: QEMU model of the GPIO device on the SiFive E300 series SOCs. The pins are not used by a board definition yet, however this implementation can already be used to trigger GPIO interrupts from the software by configuring a pin as both output and input. This has been at the top of my inbox for a while now. There was some discussion on it, but it doesn't look like there was anything conclusive so I'm OK taking it in to hw/riscv for now and then dealing with it later. Do you have anything that actually glues this to a machine so I can test it? Signed-off-by: Fabien Chouteau --- Makefile.objs | 1 + hw/riscv/Makefile.objs | 1 + hw/riscv/sifive_e.c| 28 ++- hw/riscv/sifive_gpio.c | 388 + hw/riscv/trace-events | 7 + include/hw/riscv/sifive_e.h| 8 +- include/hw/riscv/sifive_gpio.h | 72 ++ 7 files changed, 501 insertions(+), 4 deletions(-) create mode 100644 hw/riscv/sifive_gpio.c create mode 100644 hw/riscv/trace-events create mode 100644 include/hw/riscv/sifive_gpio.h diff --git a/Makefile.objs b/Makefile.objs index 67a054b08a..d40eb089ae 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -184,6 +184,7 @@ trace-events-subdirs += hw/virtio trace-events-subdirs += hw/watchdog trace-events-subdirs += hw/xen trace-events-subdirs += hw/gpio +trace-events-subdirs += hw/riscv trace-events-subdirs += io trace-events-subdirs += linux-user trace-events-subdirs += migration diff --git a/hw/riscv/Makefile.objs b/hw/riscv/Makefile.objs index 1dde01d39d..ced7935371 100644 --- a/hw/riscv/Makefile.objs +++ b/hw/riscv/Makefile.objs @@ -7,5 +7,6 @@ obj-y += sifive_plic.o obj-y += sifive_test.o obj-y += sifive_u.o obj-y += sifive_uart.o +obj-y += sifive_gpio.o obj-y += spike.o obj-y += virt.o diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c index 5d9d65ff29..49c1dd986c 100644 --- a/hw/riscv/sifive_e.c +++ b/hw/riscv/sifive_e.c @@ -146,11 +146,15 @@ static void riscv_sifive_e_soc_init(Object *obj) &error_abort); object_property_set_int(OBJECT(&s->cpus), smp_cpus, "num-harts", &error_abort); +sysbus_init_child_obj(obj, "riscv.sifive.e.gpio0", + &s->gpio, sizeof(s->gpio), + TYPE_SIFIVE_GPIO); } static void riscv_sifive_e_soc_realize(DeviceState *dev, Error **errp) { const struct MemmapEntry *memmap = sifive_e_memmap; +Error *err = NULL; SiFiveESoCState *s = RISCV_E_SOC(dev); MemoryRegion *sys_mem = get_system_memory(); @@ -184,8 +188,28 @@ static void riscv_sifive_e_soc_realize(DeviceState *dev, Error **errp) sifive_mmio_emulate(sys_mem, "riscv.sifive.e.aon", memmap[SIFIVE_E_AON].base, memmap[SIFIVE_E_AON].size); sifive_prci_create(memmap[SIFIVE_E_PRCI].base); -sifive_mmio_emulate(sys_mem, "riscv.sifive.e.gpio0", -memmap[SIFIVE_E_GPIO0].base, memmap[SIFIVE_E_GPIO0].size); + +/* GPIO */ + +object_property_set_bool(OBJECT(&s->gpio), true, "realized", &err); +if (err) { +error_propagate(errp, err); +return; +} + +/* Map GPIO registers */ +sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio), 0, memmap[SIFIVE_E_GPIO0].base); + +/* Pass all GPIOs to the SOC layer so they are available to the board */ +qdev_pass_gpios(DEVICE(&s->gpio), dev, NULL); + +/* Connect GPIO interrupts to the PLIC */ +for (int i = 0; i < 32; i++) { +sysbus_connect_irq(SYS_BUS_DEVICE(&s->gpio), i, + qdev_get_gpio_in(DEVICE(s->plic), +SIFIVE_E_GPIO0_IRQ0 + i)); +} + sifive_uart_create(sys_mem, memmap[SIFIVE_E_UART0].base, serial_hd(0), qdev_get_gpio_in(DEVICE(s->plic), SIFIVE_E_UART0_IRQ)); sifive_mmio_emulate(sys_mem, "riscv.sifive.e.qspi0", diff --git a/hw/riscv/sifive_gpio.c b/hw/riscv/sifive_gpio.c new file mode 100644 index 00..06bd8112d7 --- /dev/null +++ b/hw/riscv/sifive_gpio.c @@ -0,0 +1,388 @@ +/* + * sifive System-on-Chip general purpose input/output register definition + * + * Copyright 2019 AdaCore + * + * Base on nrf51_gpio.c: + * + * Copyright 2018 Steffen Görtz + * + * This code is licensed under the GPL version 2 or later. See + * the COPYING file in the top-level directory. + */ + +#include "qemu/osdep.h" +#include "qemu/log.h" +#include "hw/riscv/sifive_gpio.h" +#include "trace.h" + +static void update_output_irq(SIFIVEGPIOState *s) +{ + +uint32_t pending; +uint32_t pin; + +pending = s->high_ip & s->high_ie; +pending |= s->low_ip & s->low_ie; +pending |= s->rise_ip & s->rise_ie; +pending |= s->fall_ip & s->fall_ie; + +for (int i = 0; i < SIFIVE_GPIO_PINS; i++) { +pin = 1 << i; +qemu_set_irq(s->irq[i], (pending & pin) != 0); +trace_sifive_gpio_update_output_irq(i, (pend
Re: [Qemu-devel] [PATCH] SiFive RISC-V GPIO Device
On Wed, Feb 13, 2019 at 1:36 PM Peter Maydell wrote: > > On Wed, 13 Feb 2019 at 18:54, Thomas Huth wrote: > > > > On 2019-02-13 18:14, Peter Maydell wrote: > > > On Wed, 13 Feb 2019 at 00:13, Alistair Francis > > > wrote: > > >> I know the other RISC-V files don't do it, but this should go in the > > >> hw/gpio directory instead of hw/riscv. > > > > > > It might be nice to move those existing riscv devices into > > > their proper places at some point: should be a fairly easy > > > cleanup patch if somebody wants to take it on. (Advice > > > on what should live where available on request.) > > > > If devices only work on riscv, they should IMHO stay in hw/riscv/. I > > think hw/gpio/ and friends should primarily be used if a device is > > shared between architectures. If they stay in hw/riscv/ it is way easier > > to match the devices with wildcards in the MAINTAINERS file. Just my > > 0.02 € only, of course. > > This is not the way we generally arrange the source tree, > at least not the parts I'm familiar with. The rule of > thumb I use is that devices of type X go in hw/X, and > hw/$ARCH is only for board models and SoC container objects. > I think the model for this is the Linux kernel, which > splits drivers for devices into directories by device > family, rather than putting them all in arch/whatever. > A split that puts all the architecture-specific device > models in hw/$ARCH has the significant disadvantage that > it would put a huge number of files in hw/arm ... That is how I understood it as well. I think some of the current files in hw/riscv/ could be moved, but they are somewhat border line and we don't have many files yet so I wasn't bothering yet. This GPIO device doesn't have to be RISC-V specific so it makes sense to put it under hw/gpio. The maintainers file will need to be updated though. Alistair > > (More generally, I don't think the exact way we split > the source files between directories matters much, but > I do think it helps if we try to be consistent about it.) > > thanks > -- PMM
Re: [Qemu-devel] [PATCH] SiFive RISC-V GPIO Device
On Wed, 13 Feb 2019 at 18:54, Thomas Huth wrote: > > On 2019-02-13 18:14, Peter Maydell wrote: > > On Wed, 13 Feb 2019 at 00:13, Alistair Francis wrote: > >> I know the other RISC-V files don't do it, but this should go in the > >> hw/gpio directory instead of hw/riscv. > > > > It might be nice to move those existing riscv devices into > > their proper places at some point: should be a fairly easy > > cleanup patch if somebody wants to take it on. (Advice > > on what should live where available on request.) > > If devices only work on riscv, they should IMHO stay in hw/riscv/. I > think hw/gpio/ and friends should primarily be used if a device is > shared between architectures. If they stay in hw/riscv/ it is way easier > to match the devices with wildcards in the MAINTAINERS file. Just my > 0.02 € only, of course. This is not the way we generally arrange the source tree, at least not the parts I'm familiar with. The rule of thumb I use is that devices of type X go in hw/X, and hw/$ARCH is only for board models and SoC container objects. I think the model for this is the Linux kernel, which splits drivers for devices into directories by device family, rather than putting them all in arch/whatever. A split that puts all the architecture-specific device models in hw/$ARCH has the significant disadvantage that it would put a huge number of files in hw/arm ... (More generally, I don't think the exact way we split the source files between directories matters much, but I do think it helps if we try to be consistent about it.) thanks -- PMM
Re: [Qemu-devel] [PATCH] SiFive RISC-V GPIO Device
On Wed, 13 Feb 2019 10:54:08 PST (-0800), th...@redhat.com wrote: On 2019-02-13 18:14, Peter Maydell wrote: On Wed, 13 Feb 2019 at 00:13, Alistair Francis wrote: I know the other RISC-V files don't do it, but this should go in the hw/gpio directory instead of hw/riscv. It might be nice to move those existing riscv devices into their proper places at some point: should be a fairly easy cleanup patch if somebody wants to take it on. (Advice on what should live where available on request.) If devices only work on riscv, they should IMHO stay in hw/riscv/. I think hw/gpio/ and friends should primarily be used if a device is shared between architectures. If they stay in hw/riscv/ it is way easier to match the devices with wildcards in the MAINTAINERS file. Just my 0.02 € only, of course. FWIW, I generally consider things like SiFive's GPIO controller to have nothing to do with RISC-V and therefor don't belong in hw/riscv. That said, I don't really care that much so I'm happy with pretty much any source code organization that works for everyone else. Let me know if we should move them around.
Re: [Qemu-devel] [PATCH] SiFive RISC-V GPIO Device
On 2019-02-13 18:14, Peter Maydell wrote: > On Wed, 13 Feb 2019 at 00:13, Alistair Francis wrote: >> I know the other RISC-V files don't do it, but this should go in the >> hw/gpio directory instead of hw/riscv. > > It might be nice to move those existing riscv devices into > their proper places at some point: should be a fairly easy > cleanup patch if somebody wants to take it on. (Advice > on what should live where available on request.) If devices only work on riscv, they should IMHO stay in hw/riscv/. I think hw/gpio/ and friends should primarily be used if a device is shared between architectures. If they stay in hw/riscv/ it is way easier to match the devices with wildcards in the MAINTAINERS file. Just my 0.02 € only, of course. Thomas
Re: [Qemu-devel] [PATCH] SiFive RISC-V GPIO Device
On Wed, 13 Feb 2019 at 00:13, Alistair Francis wrote: > I know the other RISC-V files don't do it, but this should go in the > hw/gpio directory instead of hw/riscv. It might be nice to move those existing riscv devices into their proper places at some point: should be a fairly easy cleanup patch if somebody wants to take it on. (Advice on what should live where available on request.) thanks -- PMM
Re: [Qemu-devel] [PATCH] SiFive RISC-V GPIO Device
On Tue, Feb 12, 2019 at 9:39 AM Fabien Chouteau wrote: > > QEMU model of the GPIO device on the SiFive E300 series SOCs. > > The pins are not used by a board definition yet, however this > implementation can already be used to trigger GPIO interrupts from the > software by configuring a pin as both output and input. > > Signed-off-by: Fabien Chouteau Hey, Thanks for the patch! > --- > Makefile.objs | 1 + > hw/riscv/Makefile.objs | 1 + > hw/riscv/sifive_e.c| 28 ++- > hw/riscv/sifive_gpio.c | 388 + > hw/riscv/trace-events | 7 + > include/hw/riscv/sifive_e.h| 8 +- > include/hw/riscv/sifive_gpio.h | 72 ++ > 7 files changed, 501 insertions(+), 4 deletions(-) > create mode 100644 hw/riscv/sifive_gpio.c > create mode 100644 hw/riscv/trace-events > create mode 100644 include/hw/riscv/sifive_gpio.h > > diff --git a/Makefile.objs b/Makefile.objs > index 67a054b08a..d40eb089ae 100644 > --- a/Makefile.objs > +++ b/Makefile.objs > @@ -184,6 +184,7 @@ trace-events-subdirs += hw/virtio > trace-events-subdirs += hw/watchdog > trace-events-subdirs += hw/xen > trace-events-subdirs += hw/gpio > +trace-events-subdirs += hw/riscv > trace-events-subdirs += io > trace-events-subdirs += linux-user > trace-events-subdirs += migration > diff --git a/hw/riscv/Makefile.objs b/hw/riscv/Makefile.objs > index 1dde01d39d..ced7935371 100644 > --- a/hw/riscv/Makefile.objs > +++ b/hw/riscv/Makefile.objs > @@ -7,5 +7,6 @@ obj-y += sifive_plic.o > obj-y += sifive_test.o > obj-y += sifive_u.o > obj-y += sifive_uart.o > +obj-y += sifive_gpio.o I know the other RISC-V files don't do it, but this should go in the hw/gpio directory instead of hw/riscv. > obj-y += spike.o > obj-y += virt.o > diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c > index 5d9d65ff29..49c1dd986c 100644 > --- a/hw/riscv/sifive_e.c > +++ b/hw/riscv/sifive_e.c > @@ -146,11 +146,15 @@ static void riscv_sifive_e_soc_init(Object *obj) > &error_abort); > object_property_set_int(OBJECT(&s->cpus), smp_cpus, "num-harts", > &error_abort); > +sysbus_init_child_obj(obj, "riscv.sifive.e.gpio0", > + &s->gpio, sizeof(s->gpio), > + TYPE_SIFIVE_GPIO); > } > > static void riscv_sifive_e_soc_realize(DeviceState *dev, Error **errp) > { > const struct MemmapEntry *memmap = sifive_e_memmap; > +Error *err = NULL; > > SiFiveESoCState *s = RISCV_E_SOC(dev); > MemoryRegion *sys_mem = get_system_memory(); > @@ -184,8 +188,28 @@ static void riscv_sifive_e_soc_realize(DeviceState *dev, > Error **errp) > sifive_mmio_emulate(sys_mem, "riscv.sifive.e.aon", > memmap[SIFIVE_E_AON].base, memmap[SIFIVE_E_AON].size); > sifive_prci_create(memmap[SIFIVE_E_PRCI].base); > -sifive_mmio_emulate(sys_mem, "riscv.sifive.e.gpio0", > -memmap[SIFIVE_E_GPIO0].base, memmap[SIFIVE_E_GPIO0].size); > + > +/* GPIO */ > + > +object_property_set_bool(OBJECT(&s->gpio), true, "realized", &err); > +if (err) { > +error_propagate(errp, err); > +return; > +} > + > +/* Map GPIO registers */ > +sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio), 0, > memmap[SIFIVE_E_GPIO0].base); > + > +/* Pass all GPIOs to the SOC layer so they are available to the board */ > +qdev_pass_gpios(DEVICE(&s->gpio), dev, NULL); > + > +/* Connect GPIO interrupts to the PLIC */ > +for (int i = 0; i < 32; i++) { > +sysbus_connect_irq(SYS_BUS_DEVICE(&s->gpio), i, > + qdev_get_gpio_in(DEVICE(s->plic), > +SIFIVE_E_GPIO0_IRQ0 + i)); > +} > + It's common in QEMU world to split your patch in two. One that adds the device and then one that connects it. In this case the patch isn't too complex so it's fine, just for future reference. > sifive_uart_create(sys_mem, memmap[SIFIVE_E_UART0].base, > serial_hd(0), qdev_get_gpio_in(DEVICE(s->plic), SIFIVE_E_UART0_IRQ)); > sifive_mmio_emulate(sys_mem, "riscv.sifive.e.qspi0", > diff --git a/hw/riscv/sifive_gpio.c b/hw/riscv/sifive_gpio.c > new file mode 100644 > index 00..06bd8112d7 > --- /dev/null > +++ b/hw/riscv/sifive_gpio.c > @@ -0,0 +1,388 @@ > +/* > + * sifive System-on-Chip general purpose input/output register definition > + * > + * Copyright 2019 AdaCore > + * > + * Base on nrf51_gpio.c: > + * > + * Copyright 2018 Steffen Görtz > + * > + * This code is licensed under the GPL version 2 or later. See > + * the COPYING file in the top-level directory. > + */ > + > +#include "qemu/osdep.h" > +#include "qemu/log.h" > +#include "hw/riscv/sifive_gpio.h" > +#include "trace.h" > + > +static void update_output_irq(SIFIVEGPIOState *s) > +{ > + Remove the new line. > +uint32_t pending; > +uint32_t pin; > + > +pending = s->high_ip & s->high_ie; > +pendin
[Qemu-devel] [PATCH] SiFive RISC-V GPIO Device
QEMU model of the GPIO device on the SiFive E300 series SOCs. The pins are not used by a board definition yet, however this implementation can already be used to trigger GPIO interrupts from the software by configuring a pin as both output and input. Signed-off-by: Fabien Chouteau --- Makefile.objs | 1 + hw/riscv/Makefile.objs | 1 + hw/riscv/sifive_e.c| 28 ++- hw/riscv/sifive_gpio.c | 388 + hw/riscv/trace-events | 7 + include/hw/riscv/sifive_e.h| 8 +- include/hw/riscv/sifive_gpio.h | 72 ++ 7 files changed, 501 insertions(+), 4 deletions(-) create mode 100644 hw/riscv/sifive_gpio.c create mode 100644 hw/riscv/trace-events create mode 100644 include/hw/riscv/sifive_gpio.h diff --git a/Makefile.objs b/Makefile.objs index 67a054b08a..d40eb089ae 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -184,6 +184,7 @@ trace-events-subdirs += hw/virtio trace-events-subdirs += hw/watchdog trace-events-subdirs += hw/xen trace-events-subdirs += hw/gpio +trace-events-subdirs += hw/riscv trace-events-subdirs += io trace-events-subdirs += linux-user trace-events-subdirs += migration diff --git a/hw/riscv/Makefile.objs b/hw/riscv/Makefile.objs index 1dde01d39d..ced7935371 100644 --- a/hw/riscv/Makefile.objs +++ b/hw/riscv/Makefile.objs @@ -7,5 +7,6 @@ obj-y += sifive_plic.o obj-y += sifive_test.o obj-y += sifive_u.o obj-y += sifive_uart.o +obj-y += sifive_gpio.o obj-y += spike.o obj-y += virt.o diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c index 5d9d65ff29..49c1dd986c 100644 --- a/hw/riscv/sifive_e.c +++ b/hw/riscv/sifive_e.c @@ -146,11 +146,15 @@ static void riscv_sifive_e_soc_init(Object *obj) &error_abort); object_property_set_int(OBJECT(&s->cpus), smp_cpus, "num-harts", &error_abort); +sysbus_init_child_obj(obj, "riscv.sifive.e.gpio0", + &s->gpio, sizeof(s->gpio), + TYPE_SIFIVE_GPIO); } static void riscv_sifive_e_soc_realize(DeviceState *dev, Error **errp) { const struct MemmapEntry *memmap = sifive_e_memmap; +Error *err = NULL; SiFiveESoCState *s = RISCV_E_SOC(dev); MemoryRegion *sys_mem = get_system_memory(); @@ -184,8 +188,28 @@ static void riscv_sifive_e_soc_realize(DeviceState *dev, Error **errp) sifive_mmio_emulate(sys_mem, "riscv.sifive.e.aon", memmap[SIFIVE_E_AON].base, memmap[SIFIVE_E_AON].size); sifive_prci_create(memmap[SIFIVE_E_PRCI].base); -sifive_mmio_emulate(sys_mem, "riscv.sifive.e.gpio0", -memmap[SIFIVE_E_GPIO0].base, memmap[SIFIVE_E_GPIO0].size); + +/* GPIO */ + +object_property_set_bool(OBJECT(&s->gpio), true, "realized", &err); +if (err) { +error_propagate(errp, err); +return; +} + +/* Map GPIO registers */ +sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio), 0, memmap[SIFIVE_E_GPIO0].base); + +/* Pass all GPIOs to the SOC layer so they are available to the board */ +qdev_pass_gpios(DEVICE(&s->gpio), dev, NULL); + +/* Connect GPIO interrupts to the PLIC */ +for (int i = 0; i < 32; i++) { +sysbus_connect_irq(SYS_BUS_DEVICE(&s->gpio), i, + qdev_get_gpio_in(DEVICE(s->plic), +SIFIVE_E_GPIO0_IRQ0 + i)); +} + sifive_uart_create(sys_mem, memmap[SIFIVE_E_UART0].base, serial_hd(0), qdev_get_gpio_in(DEVICE(s->plic), SIFIVE_E_UART0_IRQ)); sifive_mmio_emulate(sys_mem, "riscv.sifive.e.qspi0", diff --git a/hw/riscv/sifive_gpio.c b/hw/riscv/sifive_gpio.c new file mode 100644 index 00..06bd8112d7 --- /dev/null +++ b/hw/riscv/sifive_gpio.c @@ -0,0 +1,388 @@ +/* + * sifive System-on-Chip general purpose input/output register definition + * + * Copyright 2019 AdaCore + * + * Base on nrf51_gpio.c: + * + * Copyright 2018 Steffen Görtz + * + * This code is licensed under the GPL version 2 or later. See + * the COPYING file in the top-level directory. + */ + +#include "qemu/osdep.h" +#include "qemu/log.h" +#include "hw/riscv/sifive_gpio.h" +#include "trace.h" + +static void update_output_irq(SIFIVEGPIOState *s) +{ + +uint32_t pending; +uint32_t pin; + +pending = s->high_ip & s->high_ie; +pending |= s->low_ip & s->low_ie; +pending |= s->rise_ip & s->rise_ie; +pending |= s->fall_ip & s->fall_ie; + +for (int i = 0; i < SIFIVE_GPIO_PINS; i++) { +pin = 1 << i; +qemu_set_irq(s->irq[i], (pending & pin) != 0); +trace_sifive_gpio_update_output_irq(i, (pending & pin) != 0); +} +} + +static void update_state(SIFIVEGPIOState *s) +{ +size_t i; +bool prev_ival, in, in_mask, port, out_xor, pull, output_en, input_en, +rise_ip, fall_ip, low_ip, high_ip, oval, actual_value, ival; + +for (i = 0; i < SIFIVE_GPIO_PINS; i++) { + +prev_ival = extract32(s->value, i, 1); +in= extract32(s->