| Message ID | 1363358063-23973-5-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Accepted |
| Commit | c34e120554c31d45bdfbac08a5c1d9ef92a62020 |
| Headers | show |
Peter Maydell <peter.maydell@linaro.org> writes: > Don't map the pmem and dmem RAM memory regions in the milkymist-softusb > device itself. Instead just expose them as sysbus mmio regions which > the device creator can map appropriately. This allows us to drop the > pmem_base and dmem_base properties. Instead of going via > cpu_physical_memory_read/_write when the device wants to access the > RAMs, we just keep a host pointer to the memory and use that. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > Reviewed-by: Andreas Färber <afaerber@suse.de> > Acked-by: Michael Walle <michael@walle.cc> Breaks the build: [aliguori@ccnode4 qemu]$ make CC lm32-softmmu/hw/lm32/../milkymist-softusb.o /home/aliguori/git/qemu/hw/lm32/../milkymist-softusb.c: In function ‘softusb_mouse_hid_datain’: /home/aliguori/git/qemu/hw/lm32/../milkymist-softusb.c:183:38: error: ‘m’ may be used uninitialized in this function [-Werror=maybe-uninitialized] /home/aliguori/git/qemu/hw/lm32/../milkymist-softusb.c:179:13: note: ‘m’ was declared here /home/aliguori/git/qemu/hw/lm32/../milkymist-softusb.c: In function ‘softusb_kbd_hid_datain’: /home/aliguori/git/qemu/hw/lm32/../milkymist-softusb.c:197:38: error: ‘m’ may be used uninitialized in this function [-Werror=maybe-uninitialized] /home/aliguori/git/qemu/hw/lm32/../milkymist-softusb.c:193:13: note: ‘m’ was declared here cc1: all warnings being treated as errors make[1]: *** [hw/lm32/../milkymist-softusb.o] Error 1 Regards, Anthony Liguori > --- > hw/milkymist-hw.h | 4 ++-- > hw/milkymist-softusb.c | 21 +++++++++++---------- > 2 files changed, 13 insertions(+), 12 deletions(-) > > diff --git a/hw/milkymist-hw.h b/hw/milkymist-hw.h > index aa0865f..5782e1a 100644 > --- a/hw/milkymist-hw.h > +++ b/hw/milkymist-hw.h > @@ -194,12 +194,12 @@ static inline DeviceState *milkymist_softusb_create(hwaddr base, > DeviceState *dev; > > dev = qdev_create(NULL, "milkymist-softusb"); > - qdev_prop_set_uint32(dev, "pmem_base", pmem_base); > qdev_prop_set_uint32(dev, "pmem_size", pmem_size); > - qdev_prop_set_uint32(dev, "dmem_base", dmem_base); > qdev_prop_set_uint32(dev, "dmem_size", dmem_size); > qdev_init_nofail(dev); > sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base); > + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, pmem_base); > + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, dmem_base); > sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq); > > return dev; > diff --git a/hw/milkymist-softusb.c b/hw/milkymist-softusb.c > index d911686..b279d4e 100644 > --- a/hw/milkymist-softusb.c > +++ b/hw/milkymist-softusb.c > @@ -54,10 +54,11 @@ struct MilkymistSoftUsbState { > MemoryRegion dmem; > qemu_irq irq; > > + void *pmem_ptr; > + void *dmem_ptr; > + > /* device properties */ > - uint32_t pmem_base; > uint32_t pmem_size; > - uint32_t dmem_base; > uint32_t dmem_size; > > /* device registers */ > @@ -134,7 +135,7 @@ static inline void softusb_read_dmem(MilkymistSoftUsbState *s, > return; > } > > - cpu_physical_memory_read(s->dmem_base + offset, buf, len); > + memcpy(buf, s->dmem_ptr + offset, len); > } > > static inline void softusb_write_dmem(MilkymistSoftUsbState *s, > @@ -146,7 +147,7 @@ static inline void softusb_write_dmem(MilkymistSoftUsbState *s, > return; > } > > - cpu_physical_memory_write(s->dmem_base + offset, buf, len); > + memcpy(s->dmem_ptr + offset, buf, len); > } > > static inline void softusb_read_pmem(MilkymistSoftUsbState *s, > @@ -158,7 +159,7 @@ static inline void softusb_read_pmem(MilkymistSoftUsbState *s, > return; > } > > - cpu_physical_memory_read(s->pmem_base + offset, buf, len); > + memcpy(buf, s->pmem_ptr + offset, len); > } > > static inline void softusb_write_pmem(MilkymistSoftUsbState *s, > @@ -170,7 +171,7 @@ static inline void softusb_write_pmem(MilkymistSoftUsbState *s, > return; > } > > - cpu_physical_memory_write(s->pmem_base + offset, buf, len); > + memcpy(s->pmem_ptr + offset, buf, len); > } > > static void softusb_mouse_changed(MilkymistSoftUsbState *s) > @@ -270,11 +271,13 @@ static int milkymist_softusb_init(SysBusDevice *dev) > memory_region_init_ram(&s->pmem, "milkymist-softusb.pmem", > s->pmem_size); > vmstate_register_ram_global(&s->pmem); > - sysbus_add_memory(dev, s->pmem_base, &s->pmem); > + s->pmem_ptr = memory_region_get_ram_ptr(&s->pmem); > + sysbus_init_mmio(dev, &s->pmem); > memory_region_init_ram(&s->dmem, "milkymist-softusb.dmem", > s->dmem_size); > vmstate_register_ram_global(&s->dmem); > - sysbus_add_memory(dev, s->dmem_base, &s->dmem); > + s->dmem_ptr = memory_region_get_ram_ptr(&s->dmem); > + sysbus_init_mmio(dev, &s->dmem); > > hid_init(&s->hid_kbd, HID_KEYBOARD, softusb_kbd_hid_datain); > hid_init(&s->hid_mouse, HID_MOUSE, softusb_mouse_hid_datain); > @@ -298,9 +301,7 @@ static const VMStateDescription vmstate_milkymist_softusb = { > }; > > static Property milkymist_softusb_properties[] = { > - DEFINE_PROP_UINT32("pmem_base", MilkymistSoftUsbState, pmem_base, 0xa0000000), > DEFINE_PROP_UINT32("pmem_size", MilkymistSoftUsbState, pmem_size, 0x00001000), > - DEFINE_PROP_UINT32("dmem_base", MilkymistSoftUsbState, dmem_base, 0xa0020000), > DEFINE_PROP_UINT32("dmem_size", MilkymistSoftUsbState, dmem_size, 0x00002000), > DEFINE_PROP_END_OF_LIST(), > }; > -- > 1.7.9.5
Am Donnerstag 28 März 2013, 18:55:59 schrieb Anthony Liguori: > Peter Maydell <peter.maydell@linaro.org> writes: > > Don't map the pmem and dmem RAM memory regions in the milkymist-softusb > > device itself. Instead just expose them as sysbus mmio regions which > > the device creator can map appropriately. This allows us to drop the > > pmem_base and dmem_base properties. Instead of going via > > cpu_physical_memory_read/_write when the device wants to access the > > RAMs, we just keep a host pointer to the memory and use that. > > > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > Reviewed-by: Andreas Färber <afaerber@suse.de> > > Acked-by: Michael Walle <michael@walle.cc> > > Breaks the build: > > [aliguori@ccnode4 qemu]$ make > CC lm32-softmmu/hw/lm32/../milkymist-softusb.o > /home/aliguori/git/qemu/hw/lm32/../milkymist-softusb.c: In function > ‘softusb_mouse_hid_datain’: > /home/aliguori/git/qemu/hw/lm32/../milkymist-softusb.c:183:38: error: ‘m’ > may be used uninitialized in this function [-Werror=maybe-uninitialized] > /home/aliguori/git/qemu/hw/lm32/../milkymist-softusb.c:179:13: note: ‘m’ > was declared here /home/aliguori/git/qemu/hw/lm32/../milkymist-softusb.c: > In function ‘softusb_kbd_hid_datain’: > /home/aliguori/git/qemu/hw/lm32/../milkymist-softusb.c:197:38: error: ‘m’ > may be used uninitialized in this function [-Werror=maybe-uninitialized] > /home/aliguori/git/qemu/hw/lm32/../milkymist-softusb.c:193:13: note: ‘m’ > was declared here cc1: all warnings being treated as errors > make[1]: *** [hw/lm32/../milkymist-softusb.o] Error 1 are you sure, this patch breaks the build, or was is broken before? i'll send a patch soon.
On 28 March 2013 18:31, Michael Walle <michael@walle.cc> wrote: > Am Donnerstag 28 März 2013, 18:55:59 schrieb Anthony Liguori:cc> >> >> Breaks the build: >> >> [aliguori@ccnode4 qemu]$ make >> CC lm32-softmmu/hw/lm32/../milkymist-softusb.o >> /home/aliguori/git/qemu/hw/lm32/../milkymist-softusb.c: In function >> ‘softusb_mouse_hid_datain’: >> /home/aliguori/git/qemu/hw/lm32/../milkymist-softusb.c:183:38: error: ‘m’ >> may be used uninitialized in this function [-Werror=maybe-uninitialized] >> /home/aliguori/git/qemu/hw/lm32/../milkymist-softusb.c:179:13: note: ‘m’ >> was declared here /home/aliguori/git/qemu/hw/lm32/../milkymist-softusb.c: >> In function ‘softusb_kbd_hid_datain’: >> /home/aliguori/git/qemu/hw/lm32/../milkymist-softusb.c:197:38: error: ‘m’ >> may be used uninitialized in this function [-Werror=maybe-uninitialized] >> /home/aliguori/git/qemu/hw/lm32/../milkymist-softusb.c:193:13: note: ‘m’ >> was declared here cc1: all warnings being treated as errors >> make[1]: *** [hw/lm32/../milkymist-softusb.o] Error 1 > > are you sure, this patch breaks the build, or was is broken before? > > i'll send a patch soon. My compiler doesn't complain. I suspect you may be right and the problem was already there. Anyway I think that adding a 'memset(buf, 0, len);' after the error_report() calls in softusb_read_pmem()/softusb_read_dmem() will fix it. -- PMM
Am Donnerstag 28 März 2013, 19:33:48 schrieb Peter Maydell: > On 28 March 2013 18:31, Michael Walle <michael@walle.cc> wrote: > > Am Donnerstag 28 März 2013, 18:55:59 schrieb Anthony Liguori:cc> > > > >> Breaks the build: > >> > >> [aliguori@ccnode4 qemu]$ make > >> > >> CC lm32-softmmu/hw/lm32/../milkymist-softusb.o > >> > >> /home/aliguori/git/qemu/hw/lm32/../milkymist-softusb.c: In function > >> ‘softusb_mouse_hid_datain’: > >> /home/aliguori/git/qemu/hw/lm32/../milkymist-softusb.c:183:38: error: > >> ‘m’ may be used uninitialized in this function > >> [-Werror=maybe-uninitialized] > >> /home/aliguori/git/qemu/hw/lm32/../milkymist-softusb.c:179:13: note: > >> ‘m’ was declared here > >> /home/aliguori/git/qemu/hw/lm32/../milkymist-softusb.c: In function > >> ‘softusb_kbd_hid_datain’: > >> /home/aliguori/git/qemu/hw/lm32/../milkymist-softusb.c:197:38: error: > >> ‘m’ may be used uninitialized in this function > >> [-Werror=maybe-uninitialized] > >> /home/aliguori/git/qemu/hw/lm32/../milkymist-softusb.c:193:13: note: > >> ‘m’ was declared here cc1: all warnings being treated as errors > >> make[1]: *** [hw/lm32/../milkymist-softusb.o] Error 1 > > > > are you sure, this patch breaks the build, or was is broken before? > > > > i'll send a patch soon. > > My compiler doesn't complain. I suspect you may be right and > the problem was already there. Anyway I think that adding > a 'memset(buf, 0, len);' after the error_report() calls > in softusb_read_pmem()/softusb_read_dmem() will fix it. Sounds good. Are you fixing it, or should i send a patch?
diff --git a/hw/milkymist-hw.h b/hw/milkymist-hw.h index aa0865f..5782e1a 100644 --- a/hw/milkymist-hw.h +++ b/hw/milkymist-hw.h @@ -194,12 +194,12 @@ static inline DeviceState *milkymist_softusb_create(hwaddr base, DeviceState *dev; dev = qdev_create(NULL, "milkymist-softusb"); - qdev_prop_set_uint32(dev, "pmem_base", pmem_base); qdev_prop_set_uint32(dev, "pmem_size", pmem_size); - qdev_prop_set_uint32(dev, "dmem_base", dmem_base); qdev_prop_set_uint32(dev, "dmem_size", dmem_size); qdev_init_nofail(dev); sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base); + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, pmem_base); + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, dmem_base); sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq); return dev; diff --git a/hw/milkymist-softusb.c b/hw/milkymist-softusb.c index d911686..b279d4e 100644 --- a/hw/milkymist-softusb.c +++ b/hw/milkymist-softusb.c @@ -54,10 +54,11 @@ struct MilkymistSoftUsbState { MemoryRegion dmem; qemu_irq irq; + void *pmem_ptr; + void *dmem_ptr; + /* device properties */ - uint32_t pmem_base; uint32_t pmem_size; - uint32_t dmem_base; uint32_t dmem_size; /* device registers */ @@ -134,7 +135,7 @@ static inline void softusb_read_dmem(MilkymistSoftUsbState *s, return; } - cpu_physical_memory_read(s->dmem_base + offset, buf, len); + memcpy(buf, s->dmem_ptr + offset, len); } static inline void softusb_write_dmem(MilkymistSoftUsbState *s, @@ -146,7 +147,7 @@ static inline void softusb_write_dmem(MilkymistSoftUsbState *s, return; } - cpu_physical_memory_write(s->dmem_base + offset, buf, len); + memcpy(s->dmem_ptr + offset, buf, len); } static inline void softusb_read_pmem(MilkymistSoftUsbState *s, @@ -158,7 +159,7 @@ static inline void softusb_read_pmem(MilkymistSoftUsbState *s, return; } - cpu_physical_memory_read(s->pmem_base + offset, buf, len); + memcpy(buf, s->pmem_ptr + offset, len); } static inline void softusb_write_pmem(MilkymistSoftUsbState *s, @@ -170,7 +171,7 @@ static inline void softusb_write_pmem(MilkymistSoftUsbState *s, return; } - cpu_physical_memory_write(s->pmem_base + offset, buf, len); + memcpy(s->pmem_ptr + offset, buf, len); } static void softusb_mouse_changed(MilkymistSoftUsbState *s) @@ -270,11 +271,13 @@ static int milkymist_softusb_init(SysBusDevice *dev) memory_region_init_ram(&s->pmem, "milkymist-softusb.pmem", s->pmem_size); vmstate_register_ram_global(&s->pmem); - sysbus_add_memory(dev, s->pmem_base, &s->pmem); + s->pmem_ptr = memory_region_get_ram_ptr(&s->pmem); + sysbus_init_mmio(dev, &s->pmem); memory_region_init_ram(&s->dmem, "milkymist-softusb.dmem", s->dmem_size); vmstate_register_ram_global(&s->dmem); - sysbus_add_memory(dev, s->dmem_base, &s->dmem); + s->dmem_ptr = memory_region_get_ram_ptr(&s->dmem); + sysbus_init_mmio(dev, &s->dmem); hid_init(&s->hid_kbd, HID_KEYBOARD, softusb_kbd_hid_datain); hid_init(&s->hid_mouse, HID_MOUSE, softusb_mouse_hid_datain); @@ -298,9 +301,7 @@ static const VMStateDescription vmstate_milkymist_softusb = { }; static Property milkymist_softusb_properties[] = { - DEFINE_PROP_UINT32("pmem_base", MilkymistSoftUsbState, pmem_base, 0xa0000000), DEFINE_PROP_UINT32("pmem_size", MilkymistSoftUsbState, pmem_size, 0x00001000), - DEFINE_PROP_UINT32("dmem_base", MilkymistSoftUsbState, dmem_base, 0xa0020000), DEFINE_PROP_UINT32("dmem_size", MilkymistSoftUsbState, dmem_size, 0x00002000), DEFINE_PROP_END_OF_LIST(), };