| Message ID | f43fc5581003241305h2947ea46o158343b6ceb80297@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Mar 24, 2010 at 10:05:10PM +0200, Blue Swirl wrote: > On 3/24/10, Michael S. Tsirkin <mst@redhat.com> wrote: > > On Tue, Mar 23, 2010 at 11:43:51PM +0200, Blue Swirl wrote: > > > rtl8139.c, e1000.c: need to convert ldl/stl to cpu_physical_memory_read/write. > > > > > > I don't see how it would help. These still get target_phys_addr_t which > > is per-target. Further, a ton of devices do > > cpu_register_physical_memory/qemu_register_coalesced_mmio. > > These are also per target. > > I don't know what I was eating yesterday: there are no references to > ldl/stl in either rtl8139 or e1000. In fact, the conversion is simple > for the device itself, just add a property "be". The attached patch > performs this part. > > But now there is a bigger problem, how to pass the property to the > device. It's not fair to require the user to remember to set it. I still don't fully understand how come e1000 cares about target endianness. > > A simple solution would be to change all of cpu_XX functions to > > get a 64 bit address. This is a lot of churn, if we do this > > anyway we should also pass length to callbacks, this way > > rwhandler will get very small or go away completely. > > It's not too much effort to keep the target_phys_addr_t type. I don't understand - target_phys_addr_t is different for different targets to we will need to recompile the code for each target. What am I missing? > From e0ab5cc41c68207be558ccb330f4fb83fba4ee6f Mon Sep 17 00:00:00 2001 > From: Blue Swirl <blauwirbel@gmail.com> > Date: Wed, 24 Mar 2010 19:54:05 +0000 > Subject: [PATCH] Compile rtl8139 and e1000 only once > > WIP > > Signed-off-by: Blue Swirl <blauwirbel@gmail.com> > --- > Makefile.objs | 2 + > Makefile.target | 4 -- > hw/e1000.c | 108 ++++++++++++++++++++++++++++++++++++++++++------------ > hw/rtl8139.c | 82 +++++++++++++++++++++++++++++++----------- > 4 files changed, 147 insertions(+), 49 deletions(-) > > diff --git a/Makefile.objs b/Makefile.objs > index 281f7a6..54895f8 100644 > --- a/Makefile.objs > +++ b/Makefile.objs > @@ -155,6 +155,8 @@ hw-obj-y += msix.o > hw-obj-y += ne2000.o > hw-obj-y += eepro100.o > hw-obj-y += pcnet.o > +hw-obj-y += rtl8139.o > +hw-obj-y += e1000.o > > hw-obj-$(CONFIG_SMC91C111) += smc91c111.o > hw-obj-$(CONFIG_LAN9118) += lan9118.o > diff --git a/Makefile.target b/Makefile.target > index eb4d010..1a86fc4 100644 > --- a/Makefile.target > +++ b/Makefile.target > @@ -176,10 +176,6 @@ QEMU_CFLAGS += $(VNC_SASL_CFLAGS) > # xen backend driver support > obj-$(CONFIG_XEN) += xen_machine_pv.o xen_domainbuild.o > > -# PCI network cards > -obj-y += rtl8139.o > -obj-y += e1000.o > - > # Hardware support > obj-i386-y = ide/core.o > obj-i386-y += pckbd.o dma.o > diff --git a/hw/e1000.c b/hw/e1000.c > index fd3059a..0f72db8 100644 > --- a/hw/e1000.c > +++ b/hw/e1000.c > @@ -121,6 +121,7 @@ typedef struct E1000State_st { > uint16_t reading; > uint32_t old_eecd; > } eecd_state; > + uint32_t be; > } E1000State; > > #define defreg(x) x = (E1000_##x>>2) > @@ -825,14 +826,11 @@ static void (*macreg_writeops[])(E1000State *, int, uint32_t) = { > enum { NWRITEOPS = ARRAY_SIZE(macreg_writeops) }; > > static void > -e1000_mmio_writel(void *opaque, target_phys_addr_t addr, uint32_t val) > +e1000_mmio_writel_le(void *opaque, target_phys_addr_t addr, uint32_t val) > { > E1000State *s = opaque; > unsigned int index = (addr & 0x1ffff) >> 2; > > -#ifdef TARGET_WORDS_BIGENDIAN > - val = bswap32(val); > -#endif > if (index < NWRITEOPS && macreg_writeops[index]) > macreg_writeops[index](s, index, val); > else if (index < NREADOPS && macreg_readops[index]) > @@ -841,25 +839,47 @@ e1000_mmio_writel(void *opaque, target_phys_addr_t addr, uint32_t val) > DBGOUT(UNKNOWN, "MMIO unknown write addr=0x%08x,val=0x%08x\n", > index<<2, val); > } > +static void > +e1000_mmio_writel_be(void *opaque, target_phys_addr_t addr, uint32_t val) > +{ > + val = bswap32(val); > + e1000_mmio_writel_le(opaque, addr, val); > +} > > static void > -e1000_mmio_writew(void *opaque, target_phys_addr_t addr, uint32_t val) > +e1000_mmio_writew_le(void *opaque, target_phys_addr_t addr, uint32_t val) > { > // emulate hw without byte enables: no RMW > - e1000_mmio_writel(opaque, addr & ~3, > - (val & 0xffff) << (8*(addr & 3))); > + e1000_mmio_writel_le(opaque, addr & ~3, > + (val & 0xffff) << (8*(addr & 3))); > } > > static void > -e1000_mmio_writeb(void *opaque, target_phys_addr_t addr, uint32_t val) > +e1000_mmio_writew_be(void *opaque, target_phys_addr_t addr, uint32_t val) > { > // emulate hw without byte enables: no RMW > - e1000_mmio_writel(opaque, addr & ~3, > - (val & 0xff) << (8*(addr & 3))); > + e1000_mmio_writel_be(opaque, addr & ~3, > + (val & 0xffff) << (8*(addr & 3))); > +} > + > +static void > +e1000_mmio_writeb_be(void *opaque, target_phys_addr_t addr, uint32_t val) > +{ > + // emulate hw without byte enables: no RMW > + e1000_mmio_writel_be(opaque, addr & ~3, > + (val & 0xff) << (8*(addr & 3))); > +} > + > +static void > +e1000_mmio_writeb_le(void *opaque, target_phys_addr_t addr, uint32_t val) > +{ > + // emulate hw without byte enables: no RMW > + e1000_mmio_writel_le(opaque, addr & ~3, > + (val & 0xff) << (8*(addr & 3))); > } > > static uint32_t > -e1000_mmio_readl(void *opaque, target_phys_addr_t addr) > +e1000_mmio_readl_le(void *opaque, target_phys_addr_t addr) > { > E1000State *s = opaque; > unsigned int index = (addr & 0x1ffff) >> 2; > @@ -867,9 +887,6 @@ e1000_mmio_readl(void *opaque, target_phys_addr_t addr) > if (index < NREADOPS && macreg_readops[index]) > { > uint32_t val = macreg_readops[index](s, index); > -#ifdef TARGET_WORDS_BIGENDIAN > - val = bswap32(val); > -#endif > return val; > } > DBGOUT(UNKNOWN, "MMIO unknown read addr=0x%08x\n", index<<2); > @@ -877,16 +894,38 @@ e1000_mmio_readl(void *opaque, target_phys_addr_t addr) > } > > static uint32_t > -e1000_mmio_readb(void *opaque, target_phys_addr_t addr) > +e1000_mmio_readl_be(void *opaque, target_phys_addr_t addr) > { > - return ((e1000_mmio_readl(opaque, addr & ~3)) >> > + uint32_t val = e1000_mmio_readl_le(opaque, addr); > + val = bswap32(val); > + return val; > +} > + > +static uint32_t > +e1000_mmio_readb_be(void *opaque, target_phys_addr_t addr) > +{ > + return ((e1000_mmio_readl_be(opaque, addr & ~3)) >> > (8 * (addr & 3))) & 0xff; > } > > static uint32_t > -e1000_mmio_readw(void *opaque, target_phys_addr_t addr) > +e1000_mmio_readb_le(void *opaque, target_phys_addr_t addr) > +{ > + return ((e1000_mmio_readl_le(opaque, addr & ~3)) >> > + (8 * (addr & 3))) & 0xff; > +} > + > +static uint32_t > +e1000_mmio_readw_le(void *opaque, target_phys_addr_t addr) > +{ > + return ((e1000_mmio_readl_le(opaque, addr & ~3)) >> > + (8 * (addr & 3))) & 0xffff; > +} > + > +static uint32_t > +e1000_mmio_readw_be(void *opaque, target_phys_addr_t addr) > { > - return ((e1000_mmio_readl(opaque, addr & ~3)) >> > + return ((e1000_mmio_readl_be(opaque, addr & ~3)) >> > (8 * (addr & 3))) & 0xffff; > } > > @@ -1008,13 +1047,28 @@ static const uint32_t mac_reg_init[] = { > }; > > /* PCI interface */ > +static CPUWriteMemoryFunc * const e1000_mmio_write_be[] = { > + e1000_mmio_writeb_be, > + e1000_mmio_writew_be, > + e1000_mmio_writel_be > +}; > > -static CPUWriteMemoryFunc * const e1000_mmio_write[] = { > - e1000_mmio_writeb, e1000_mmio_writew, e1000_mmio_writel > +static CPUReadMemoryFunc * const e1000_mmio_read_be[] = { > + e1000_mmio_readb_be, > + e1000_mmio_readw_be, > + e1000_mmio_readl_be > }; > > -static CPUReadMemoryFunc * const e1000_mmio_read[] = { > - e1000_mmio_readb, e1000_mmio_readw, e1000_mmio_readl > +static CPUWriteMemoryFunc * const e1000_mmio_write_le[] = { > + e1000_mmio_writeb_le, > + e1000_mmio_writew_le, > + e1000_mmio_writel_le > +}; > + > +static CPUReadMemoryFunc * const e1000_mmio_read_le[] = { > + e1000_mmio_readb_le, > + e1000_mmio_readw_le, > + e1000_mmio_readl_le > }; > > static void > @@ -1102,8 +1156,13 @@ static int pci_e1000_init(PCIDevice *pci_dev) > /* TODO: RST# value should be 0 if programmable, PCI spec 6.2.4 */ > pci_conf[PCI_INTERRUPT_PIN] = 1; // interrupt pin 0 > > - d->mmio_index = cpu_register_io_memory(e1000_mmio_read, > - e1000_mmio_write, d); > + if (d->be) { > + d->mmio_index = cpu_register_io_memory(e1000_mmio_read_be, > + e1000_mmio_write_be, d); > + } else { > + d->mmio_index = cpu_register_io_memory(e1000_mmio_read_le, > + e1000_mmio_write_le, d); > + } > > pci_register_bar((PCIDevice *)d, 0, PNPMMIO_SIZE, > PCI_BASE_ADDRESS_SPACE_MEMORY, e1000_mmio_map); > @@ -1146,6 +1205,7 @@ static PCIDeviceInfo e1000_info = { > .romfile = "pxe-e1000.bin", > .qdev.props = (Property[]) { > DEFINE_NIC_PROPERTIES(E1000State, conf), > + DEFINE_PROP_UINT32("be", E1000State, be, 0), > DEFINE_PROP_END_OF_LIST(), > } > }; > diff --git a/hw/rtl8139.c b/hw/rtl8139.c > index 72e2242..ef5f1fd 100644 > --- a/hw/rtl8139.c > +++ b/hw/rtl8139.c > @@ -493,7 +493,7 @@ typedef struct RTL8139State { > /* PCI interrupt timer */ > QEMUTimer *timer; > int64_t TimerExpire; > - > + uint32_t be; > } RTL8139State; > > static void rtl8139_set_next_tctr_time(RTL8139State *s, int64_t current_time); > @@ -3123,19 +3123,29 @@ static void rtl8139_mmio_writeb(void *opaque, target_phys_addr_t addr, uint32_t > rtl8139_io_writeb(opaque, addr & 0xFF, val); > } > > -static void rtl8139_mmio_writew(void *opaque, target_phys_addr_t addr, uint32_t val) > +static void rtl8139_mmio_writew_be(void *opaque, target_phys_addr_t addr, > + uint32_t val) > { > -#ifdef TARGET_WORDS_BIGENDIAN > val = bswap16(val); > -#endif > rtl8139_io_writew(opaque, addr & 0xFF, val); > } > > -static void rtl8139_mmio_writel(void *opaque, target_phys_addr_t addr, uint32_t val) > +static void rtl8139_mmio_writew_le(void *opaque, target_phys_addr_t addr, > + uint32_t val) > +{ > + rtl8139_io_writew(opaque, addr & 0xFF, val); > +} > + > +static void rtl8139_mmio_writel_be(void *opaque, target_phys_addr_t addr, > + uint32_t val) > { > -#ifdef TARGET_WORDS_BIGENDIAN > val = bswap32(val); > -#endif > + rtl8139_io_writel(opaque, addr & 0xFF, val); > +} > + > +static void rtl8139_mmio_writel_le(void *opaque, target_phys_addr_t addr, > + uint32_t val) > +{ > rtl8139_io_writel(opaque, addr & 0xFF, val); > } > > @@ -3144,21 +3154,31 @@ static uint32_t rtl8139_mmio_readb(void *opaque, target_phys_addr_t addr) > return rtl8139_io_readb(opaque, addr & 0xFF); > } > > -static uint32_t rtl8139_mmio_readw(void *opaque, target_phys_addr_t addr) > +static uint32_t rtl8139_mmio_readw_be(void *opaque, target_phys_addr_t addr) > { > uint32_t val = rtl8139_io_readw(opaque, addr & 0xFF); > -#ifdef TARGET_WORDS_BIGENDIAN > val = bswap16(val); > -#endif > return val; > } > > -static uint32_t rtl8139_mmio_readl(void *opaque, target_phys_addr_t addr) > +static uint32_t rtl8139_mmio_readw_le(void *opaque, target_phys_addr_t addr) > +{ > + uint32_t val = rtl8139_io_readw(opaque, addr & 0xFF); > + > + return val; > +} > + > +static uint32_t rtl8139_mmio_readl_be(void *opaque, target_phys_addr_t addr) > { > uint32_t val = rtl8139_io_readl(opaque, addr & 0xFF); > -#ifdef TARGET_WORDS_BIGENDIAN > val = bswap32(val); > -#endif > + return val; > +} > + > +static uint32_t rtl8139_mmio_readl_le(void *opaque, target_phys_addr_t addr) > +{ > + uint32_t val = rtl8139_io_readl(opaque, addr & 0xFF); > + > return val; > } > > @@ -3292,16 +3312,28 @@ static void rtl8139_ioport_map(PCIDevice *pci_dev, int region_num, > register_ioport_read( addr, 0x100, 4, rtl8139_ioport_readl, s); > } > > -static CPUReadMemoryFunc * const rtl8139_mmio_read[3] = { > +static CPUReadMemoryFunc * const rtl8139_mmio_read_be[3] = { > rtl8139_mmio_readb, > - rtl8139_mmio_readw, > - rtl8139_mmio_readl, > + rtl8139_mmio_readw_be, > + rtl8139_mmio_readl_be, > }; > > -static CPUWriteMemoryFunc * const rtl8139_mmio_write[3] = { > +static CPUWriteMemoryFunc * const rtl8139_mmio_write_be[3] = { > rtl8139_mmio_writeb, > - rtl8139_mmio_writew, > - rtl8139_mmio_writel, > + rtl8139_mmio_writew_be, > + rtl8139_mmio_writel_be, > +}; > + > +static CPUReadMemoryFunc * const rtl8139_mmio_read_le[3] = { > + rtl8139_mmio_readb, > + rtl8139_mmio_readw_le, > + rtl8139_mmio_readl_le, > +}; > + > +static CPUWriteMemoryFunc * const rtl8139_mmio_write_le[3] = { > + rtl8139_mmio_writeb, > + rtl8139_mmio_writew_le, > + rtl8139_mmio_writel_le, > }; > > static void rtl8139_timer(void *opaque) > @@ -3369,8 +3401,15 @@ static int pci_rtl8139_init(PCIDevice *dev) > pci_conf[PCI_CAPABILITY_LIST] = 0xdc; > > /* I/O handler for memory-mapped I/O */ > - s->rtl8139_mmio_io_addr = > - cpu_register_io_memory(rtl8139_mmio_read, rtl8139_mmio_write, s); > + if (s->be) { > + s->rtl8139_mmio_io_addr = > + cpu_register_io_memory(rtl8139_mmio_read_be, rtl8139_mmio_write_be, > + s); > + } else { > + s->rtl8139_mmio_io_addr = > + cpu_register_io_memory(rtl8139_mmio_read_le, rtl8139_mmio_write_le, > + s); > + } > > pci_register_bar(&s->dev, 0, 0x100, > PCI_BASE_ADDRESS_SPACE_IO, rtl8139_ioport_map); > @@ -3404,6 +3443,7 @@ static PCIDeviceInfo rtl8139_info = { > .romfile = "pxe-rtl8139.bin", > .qdev.props = (Property[]) { > DEFINE_NIC_PROPERTIES(RTL8139State, conf), > + DEFINE_PROP_UINT32("be", RTL8139State, be, 0), > DEFINE_PROP_END_OF_LIST(), > } > }; > -- > 1.5.6.5 >
On 3/24/10, Michael S. Tsirkin <mst@redhat.com> wrote: > On Wed, Mar 24, 2010 at 10:05:10PM +0200, Blue Swirl wrote: > > On 3/24/10, Michael S. Tsirkin <mst@redhat.com> wrote: > > > On Tue, Mar 23, 2010 at 11:43:51PM +0200, Blue Swirl wrote: > > > > rtl8139.c, e1000.c: need to convert ldl/stl to cpu_physical_memory_read/write. > > > > > > > > > I don't see how it would help. These still get target_phys_addr_t which > > > is per-target. Further, a ton of devices do > > > cpu_register_physical_memory/qemu_register_coalesced_mmio. > > > These are also per target. > > > > I don't know what I was eating yesterday: there are no references to > > ldl/stl in either rtl8139 or e1000. In fact, the conversion is simple > > for the device itself, just add a property "be". The attached patch > > performs this part. > > > > But now there is a bigger problem, how to pass the property to the > > device. It's not fair to require the user to remember to set it. > > > I still don't fully understand how come e1000 cares about > target endianness. It shouldn't. Maybe the real fix is to remove the byte swapping. > > > A simple solution would be to change all of cpu_XX functions to > > > get a 64 bit address. This is a lot of churn, if we do this > > > anyway we should also pass length to callbacks, this way > > > rwhandler will get very small or go away completely. > > > > It's not too much effort to keep the target_phys_addr_t type. > > > I don't understand - target_phys_addr_t is different for different > targets to we will need to recompile the code for each target. > What am I missing? target_phys_addr_t is 64 bit on a 64 bit host, on a 32 bit host it's size will be either 64 or 32 bits depending on the target. So the files are compiled once on 64 bit host, twice on 32 bit host if both 32 and 64 bits targets are selected. > > From e0ab5cc41c68207be558ccb330f4fb83fba4ee6f Mon Sep 17 00:00:00 2001 > > From: Blue Swirl <blauwirbel@gmail.com> > > Date: Wed, 24 Mar 2010 19:54:05 +0000 > > Subject: [PATCH] Compile rtl8139 and e1000 only once > > > > WIP > > > > Signed-off-by: Blue Swirl <blauwirbel@gmail.com> > > --- > > Makefile.objs | 2 + > > Makefile.target | 4 -- > > hw/e1000.c | 108 ++++++++++++++++++++++++++++++++++++++++++------------ > > hw/rtl8139.c | 82 +++++++++++++++++++++++++++++++----------- > > 4 files changed, 147 insertions(+), 49 deletions(-) > > > > diff --git a/Makefile.objs b/Makefile.objs > > index 281f7a6..54895f8 100644 > > --- a/Makefile.objs > > +++ b/Makefile.objs > > @@ -155,6 +155,8 @@ hw-obj-y += msix.o > > hw-obj-y += ne2000.o > > hw-obj-y += eepro100.o > > hw-obj-y += pcnet.o > > +hw-obj-y += rtl8139.o > > +hw-obj-y += e1000.o > > > > hw-obj-$(CONFIG_SMC91C111) += smc91c111.o > > hw-obj-$(CONFIG_LAN9118) += lan9118.o > > diff --git a/Makefile.target b/Makefile.target > > index eb4d010..1a86fc4 100644 > > --- a/Makefile.target > > +++ b/Makefile.target > > @@ -176,10 +176,6 @@ QEMU_CFLAGS += $(VNC_SASL_CFLAGS) > > # xen backend driver support > > obj-$(CONFIG_XEN) += xen_machine_pv.o xen_domainbuild.o > > > > -# PCI network cards > > -obj-y += rtl8139.o > > -obj-y += e1000.o > > - > > # Hardware support > > obj-i386-y = ide/core.o > > obj-i386-y += pckbd.o dma.o > > diff --git a/hw/e1000.c b/hw/e1000.c > > index fd3059a..0f72db8 100644 > > --- a/hw/e1000.c > > +++ b/hw/e1000.c > > @@ -121,6 +121,7 @@ typedef struct E1000State_st { > > uint16_t reading; > > uint32_t old_eecd; > > } eecd_state; > > + uint32_t be; > > } E1000State; > > > > #define defreg(x) x = (E1000_##x>>2) > > @@ -825,14 +826,11 @@ static void (*macreg_writeops[])(E1000State *, int, uint32_t) = { > > enum { NWRITEOPS = ARRAY_SIZE(macreg_writeops) }; > > > > static void > > -e1000_mmio_writel(void *opaque, target_phys_addr_t addr, uint32_t val) > > +e1000_mmio_writel_le(void *opaque, target_phys_addr_t addr, uint32_t val) > > { > > E1000State *s = opaque; > > unsigned int index = (addr & 0x1ffff) >> 2; > > > > -#ifdef TARGET_WORDS_BIGENDIAN > > - val = bswap32(val); > > -#endif > > if (index < NWRITEOPS && macreg_writeops[index]) > > macreg_writeops[index](s, index, val); > > else if (index < NREADOPS && macreg_readops[index]) > > @@ -841,25 +839,47 @@ e1000_mmio_writel(void *opaque, target_phys_addr_t addr, uint32_t val) > > DBGOUT(UNKNOWN, "MMIO unknown write addr=0x%08x,val=0x%08x\n", > > index<<2, val); > > } > > +static void > > +e1000_mmio_writel_be(void *opaque, target_phys_addr_t addr, uint32_t val) > > +{ > > + val = bswap32(val); > > + e1000_mmio_writel_le(opaque, addr, val); > > +} > > > > static void > > -e1000_mmio_writew(void *opaque, target_phys_addr_t addr, uint32_t val) > > +e1000_mmio_writew_le(void *opaque, target_phys_addr_t addr, uint32_t val) > > { > > // emulate hw without byte enables: no RMW > > - e1000_mmio_writel(opaque, addr & ~3, > > - (val & 0xffff) << (8*(addr & 3))); > > + e1000_mmio_writel_le(opaque, addr & ~3, > > + (val & 0xffff) << (8*(addr & 3))); > > } > > > > static void > > -e1000_mmio_writeb(void *opaque, target_phys_addr_t addr, uint32_t val) > > +e1000_mmio_writew_be(void *opaque, target_phys_addr_t addr, uint32_t val) > > { > > // emulate hw without byte enables: no RMW > > - e1000_mmio_writel(opaque, addr & ~3, > > - (val & 0xff) << (8*(addr & 3))); > > + e1000_mmio_writel_be(opaque, addr & ~3, > > + (val & 0xffff) << (8*(addr & 3))); > > +} > > + > > +static void > > +e1000_mmio_writeb_be(void *opaque, target_phys_addr_t addr, uint32_t val) > > +{ > > + // emulate hw without byte enables: no RMW > > + e1000_mmio_writel_be(opaque, addr & ~3, > > + (val & 0xff) << (8*(addr & 3))); > > +} > > + > > +static void > > +e1000_mmio_writeb_le(void *opaque, target_phys_addr_t addr, uint32_t val) > > +{ > > + // emulate hw without byte enables: no RMW > > + e1000_mmio_writel_le(opaque, addr & ~3, > > + (val & 0xff) << (8*(addr & 3))); > > } > > > > static uint32_t > > -e1000_mmio_readl(void *opaque, target_phys_addr_t addr) > > +e1000_mmio_readl_le(void *opaque, target_phys_addr_t addr) > > { > > E1000State *s = opaque; > > unsigned int index = (addr & 0x1ffff) >> 2; > > @@ -867,9 +887,6 @@ e1000_mmio_readl(void *opaque, target_phys_addr_t addr) > > if (index < NREADOPS && macreg_readops[index]) > > { > > uint32_t val = macreg_readops[index](s, index); > > -#ifdef TARGET_WORDS_BIGENDIAN > > - val = bswap32(val); > > -#endif > > return val; > > } > > DBGOUT(UNKNOWN, "MMIO unknown read addr=0x%08x\n", index<<2); > > @@ -877,16 +894,38 @@ e1000_mmio_readl(void *opaque, target_phys_addr_t addr) > > } > > > > static uint32_t > > -e1000_mmio_readb(void *opaque, target_phys_addr_t addr) > > +e1000_mmio_readl_be(void *opaque, target_phys_addr_t addr) > > { > > - return ((e1000_mmio_readl(opaque, addr & ~3)) >> > > + uint32_t val = e1000_mmio_readl_le(opaque, addr); > > + val = bswap32(val); > > + return val; > > +} > > + > > +static uint32_t > > +e1000_mmio_readb_be(void *opaque, target_phys_addr_t addr) > > +{ > > + return ((e1000_mmio_readl_be(opaque, addr & ~3)) >> > > (8 * (addr & 3))) & 0xff; > > } > > > > static uint32_t > > -e1000_mmio_readw(void *opaque, target_phys_addr_t addr) > > +e1000_mmio_readb_le(void *opaque, target_phys_addr_t addr) > > +{ > > + return ((e1000_mmio_readl_le(opaque, addr & ~3)) >> > > + (8 * (addr & 3))) & 0xff; > > +} > > + > > +static uint32_t > > +e1000_mmio_readw_le(void *opaque, target_phys_addr_t addr) > > +{ > > + return ((e1000_mmio_readl_le(opaque, addr & ~3)) >> > > + (8 * (addr & 3))) & 0xffff; > > +} > > + > > +static uint32_t > > +e1000_mmio_readw_be(void *opaque, target_phys_addr_t addr) > > { > > - return ((e1000_mmio_readl(opaque, addr & ~3)) >> > > + return ((e1000_mmio_readl_be(opaque, addr & ~3)) >> > > (8 * (addr & 3))) & 0xffff; > > } > > > > @@ -1008,13 +1047,28 @@ static const uint32_t mac_reg_init[] = { > > }; > > > > /* PCI interface */ > > +static CPUWriteMemoryFunc * const e1000_mmio_write_be[] = { > > + e1000_mmio_writeb_be, > > + e1000_mmio_writew_be, > > + e1000_mmio_writel_be > > +}; > > > > -static CPUWriteMemoryFunc * const e1000_mmio_write[] = { > > - e1000_mmio_writeb, e1000_mmio_writew, e1000_mmio_writel > > +static CPUReadMemoryFunc * const e1000_mmio_read_be[] = { > > + e1000_mmio_readb_be, > > + e1000_mmio_readw_be, > > + e1000_mmio_readl_be > > }; > > > > -static CPUReadMemoryFunc * const e1000_mmio_read[] = { > > - e1000_mmio_readb, e1000_mmio_readw, e1000_mmio_readl > > +static CPUWriteMemoryFunc * const e1000_mmio_write_le[] = { > > + e1000_mmio_writeb_le, > > + e1000_mmio_writew_le, > > + e1000_mmio_writel_le > > +}; > > + > > +static CPUReadMemoryFunc * const e1000_mmio_read_le[] = { > > + e1000_mmio_readb_le, > > + e1000_mmio_readw_le, > > + e1000_mmio_readl_le > > }; > > > > static void > > @@ -1102,8 +1156,13 @@ static int pci_e1000_init(PCIDevice *pci_dev) > > /* TODO: RST# value should be 0 if programmable, PCI spec 6.2.4 */ > > pci_conf[PCI_INTERRUPT_PIN] = 1; // interrupt pin 0 > > > > - d->mmio_index = cpu_register_io_memory(e1000_mmio_read, > > - e1000_mmio_write, d); > > + if (d->be) { > > + d->mmio_index = cpu_register_io_memory(e1000_mmio_read_be, > > + e1000_mmio_write_be, d); > > + } else { > > + d->mmio_index = cpu_register_io_memory(e1000_mmio_read_le, > > + e1000_mmio_write_le, d); > > + } > > > > pci_register_bar((PCIDevice *)d, 0, PNPMMIO_SIZE, > > PCI_BASE_ADDRESS_SPACE_MEMORY, e1000_mmio_map); > > @@ -1146,6 +1205,7 @@ static PCIDeviceInfo e1000_info = { > > .romfile = "pxe-e1000.bin", > > .qdev.props = (Property[]) { > > DEFINE_NIC_PROPERTIES(E1000State, conf), > > + DEFINE_PROP_UINT32("be", E1000State, be, 0), > > DEFINE_PROP_END_OF_LIST(), > > } > > }; > > diff --git a/hw/rtl8139.c b/hw/rtl8139.c > > index 72e2242..ef5f1fd 100644 > > --- a/hw/rtl8139.c > > +++ b/hw/rtl8139.c > > @@ -493,7 +493,7 @@ typedef struct RTL8139State { > > /* PCI interrupt timer */ > > QEMUTimer *timer; > > int64_t TimerExpire; > > - > > + uint32_t be; > > } RTL8139State; > > > > static void rtl8139_set_next_tctr_time(RTL8139State *s, int64_t current_time); > > @@ -3123,19 +3123,29 @@ static void rtl8139_mmio_writeb(void *opaque, target_phys_addr_t addr, uint32_t > > rtl8139_io_writeb(opaque, addr & 0xFF, val); > > } > > > > -static void rtl8139_mmio_writew(void *opaque, target_phys_addr_t addr, uint32_t val) > > +static void rtl8139_mmio_writew_be(void *opaque, target_phys_addr_t addr, > > + uint32_t val) > > { > > -#ifdef TARGET_WORDS_BIGENDIAN > > val = bswap16(val); > > -#endif > > rtl8139_io_writew(opaque, addr & 0xFF, val); > > } > > > > -static void rtl8139_mmio_writel(void *opaque, target_phys_addr_t addr, uint32_t val) > > +static void rtl8139_mmio_writew_le(void *opaque, target_phys_addr_t addr, > > + uint32_t val) > > +{ > > + rtl8139_io_writew(opaque, addr & 0xFF, val); > > +} > > + > > +static void rtl8139_mmio_writel_be(void *opaque, target_phys_addr_t addr, > > + uint32_t val) > > { > > -#ifdef TARGET_WORDS_BIGENDIAN > > val = bswap32(val); > > -#endif > > + rtl8139_io_writel(opaque, addr & 0xFF, val); > > +} > > + > > +static void rtl8139_mmio_writel_le(void *opaque, target_phys_addr_t addr, > > + uint32_t val) > > +{ > > rtl8139_io_writel(opaque, addr & 0xFF, val); > > } > > > > @@ -3144,21 +3154,31 @@ static uint32_t rtl8139_mmio_readb(void *opaque, target_phys_addr_t addr) > > return rtl8139_io_readb(opaque, addr & 0xFF); > > } > > > > -static uint32_t rtl8139_mmio_readw(void *opaque, target_phys_addr_t addr) > > +static uint32_t rtl8139_mmio_readw_be(void *opaque, target_phys_addr_t addr) > > { > > uint32_t val = rtl8139_io_readw(opaque, addr & 0xFF); > > -#ifdef TARGET_WORDS_BIGENDIAN > > val = bswap16(val); > > -#endif > > return val; > > } > > > > -static uint32_t rtl8139_mmio_readl(void *opaque, target_phys_addr_t addr) > > +static uint32_t rtl8139_mmio_readw_le(void *opaque, target_phys_addr_t addr) > > +{ > > + uint32_t val = rtl8139_io_readw(opaque, addr & 0xFF); > > + > > + return val; > > +} > > + > > +static uint32_t rtl8139_mmio_readl_be(void *opaque, target_phys_addr_t addr) > > { > > uint32_t val = rtl8139_io_readl(opaque, addr & 0xFF); > > -#ifdef TARGET_WORDS_BIGENDIAN > > val = bswap32(val); > > -#endif > > + return val; > > +} > > + > > +static uint32_t rtl8139_mmio_readl_le(void *opaque, target_phys_addr_t addr) > > +{ > > + uint32_t val = rtl8139_io_readl(opaque, addr & 0xFF); > > + > > return val; > > } > > > > @@ -3292,16 +3312,28 @@ static void rtl8139_ioport_map(PCIDevice *pci_dev, int region_num, > > register_ioport_read( addr, 0x100, 4, rtl8139_ioport_readl, s); > > } > > > > -static CPUReadMemoryFunc * const rtl8139_mmio_read[3] = { > > +static CPUReadMemoryFunc * const rtl8139_mmio_read_be[3] = { > > rtl8139_mmio_readb, > > - rtl8139_mmio_readw, > > - rtl8139_mmio_readl, > > + rtl8139_mmio_readw_be, > > + rtl8139_mmio_readl_be, > > }; > > > > -static CPUWriteMemoryFunc * const rtl8139_mmio_write[3] = { > > +static CPUWriteMemoryFunc * const rtl8139_mmio_write_be[3] = { > > rtl8139_mmio_writeb, > > - rtl8139_mmio_writew, > > - rtl8139_mmio_writel, > > + rtl8139_mmio_writew_be, > > + rtl8139_mmio_writel_be, > > +}; > > + > > +static CPUReadMemoryFunc * const rtl8139_mmio_read_le[3] = { > > + rtl8139_mmio_readb, > > + rtl8139_mmio_readw_le, > > + rtl8139_mmio_readl_le, > > +}; > > + > > +static CPUWriteMemoryFunc * const rtl8139_mmio_write_le[3] = { > > + rtl8139_mmio_writeb, > > + rtl8139_mmio_writew_le, > > + rtl8139_mmio_writel_le, > > }; > > > > static void rtl8139_timer(void *opaque) > > @@ -3369,8 +3401,15 @@ static int pci_rtl8139_init(PCIDevice *dev) > > pci_conf[PCI_CAPABILITY_LIST] = 0xdc; > > > > /* I/O handler for memory-mapped I/O */ > > - s->rtl8139_mmio_io_addr = > > - cpu_register_io_memory(rtl8139_mmio_read, rtl8139_mmio_write, s); > > + if (s->be) { > > + s->rtl8139_mmio_io_addr = > > + cpu_register_io_memory(rtl8139_mmio_read_be, rtl8139_mmio_write_be, > > + s); > > + } else { > > + s->rtl8139_mmio_io_addr = > > + cpu_register_io_memory(rtl8139_mmio_read_le, rtl8139_mmio_write_le, > > + s); > > + } > > > > pci_register_bar(&s->dev, 0, 0x100, > > PCI_BASE_ADDRESS_SPACE_IO, rtl8139_ioport_map); > > @@ -3404,6 +3443,7 @@ static PCIDeviceInfo rtl8139_info = { > > .romfile = "pxe-rtl8139.bin", > > .qdev.props = (Property[]) { > > DEFINE_NIC_PROPERTIES(RTL8139State, conf), > > + DEFINE_PROP_UINT32("be", RTL8139State, be, 0), > > DEFINE_PROP_END_OF_LIST(), > > } > > }; > > -- > > 1.5.6.5 > > > >
On Wed, Mar 24, 2010 at 10:24:12PM +0200, Blue Swirl wrote: > On 3/24/10, Michael S. Tsirkin <mst@redhat.com> wrote: > > On Wed, Mar 24, 2010 at 10:05:10PM +0200, Blue Swirl wrote: > > > On 3/24/10, Michael S. Tsirkin <mst@redhat.com> wrote: > > > > On Tue, Mar 23, 2010 at 11:43:51PM +0200, Blue Swirl wrote: > > > > > rtl8139.c, e1000.c: need to convert ldl/stl to cpu_physical_memory_read/write. > > > > > > > > > > > > I don't see how it would help. These still get target_phys_addr_t which > > > > is per-target. Further, a ton of devices do > > > > cpu_register_physical_memory/qemu_register_coalesced_mmio. > > > > These are also per target. > > > > > > I don't know what I was eating yesterday: there are no references to > > > ldl/stl in either rtl8139 or e1000. In fact, the conversion is simple > > > for the device itself, just add a property "be". The attached patch > > > performs this part. > > > > > > But now there is a bigger problem, how to pass the property to the > > > device. It's not fair to require the user to remember to set it. > > > > > > I still don't fully understand how come e1000 cares about > > target endianness. > > It shouldn't. Maybe the real fix is to remove the byte swapping. Presumably it's there for a reason? > > > > A simple solution would be to change all of cpu_XX functions to > > > > get a 64 bit address. This is a lot of churn, if we do this > > > > anyway we should also pass length to callbacks, this way > > > > rwhandler will get very small or go away completely. > > > > > > It's not too much effort to keep the target_phys_addr_t type. > > > > > > I don't understand - target_phys_addr_t is different for different > > targets to we will need to recompile the code for each target. > > What am I missing? > > target_phys_addr_t is 64 bit on a 64 bit host, on a 32 bit host it's > size will be either 64 or 32 bits depending on the target. So the > files are compiled once on 64 bit host, twice on 32 bit host if both > 32 and 64 bits targets are selected. How about just making it 64 bit unconditionally? How much do we save by using a 32 bit address value?
On 03/24/2010 03:24 PM, Blue Swirl wrote: > On 3/24/10, Michael S. Tsirkin<mst@redhat.com> wrote: > >> On Wed, Mar 24, 2010 at 10:05:10PM +0200, Blue Swirl wrote: >> > On 3/24/10, Michael S. Tsirkin<mst@redhat.com> wrote: >> > > On Tue, Mar 23, 2010 at 11:43:51PM +0200, Blue Swirl wrote: >> > > > rtl8139.c, e1000.c: need to convert ldl/stl to cpu_physical_memory_read/write. >> > > >> > > >> > > I don't see how it would help. These still get target_phys_addr_t which >> > > is per-target. Further, a ton of devices do >> > > cpu_register_physical_memory/qemu_register_coalesced_mmio. >> > > These are also per target. >> > >> > I don't know what I was eating yesterday: there are no references to >> > ldl/stl in either rtl8139 or e1000. In fact, the conversion is simple >> > for the device itself, just add a property "be". The attached patch >> > performs this part. >> > >> > But now there is a bigger problem, how to pass the property to the >> > device. It's not fair to require the user to remember to set it. >> >> >> I still don't fully understand how come e1000 cares about >> target endianness. >> > It shouldn't. Maybe the real fix is to remove the byte swapping. > My previous pci memory functions patches removed the byte swapping. The problem is that PCI devices are going to operate in little endian mode (usually) whereas the CPU may be acting in big endian mode. We need to do a byte swap somewhere but the better place to do it is in the PCI bus layer. Regards, Anthony Liguori
On 3/24/10, Michael S. Tsirkin <mst@redhat.com> wrote: > On Wed, Mar 24, 2010 at 10:24:12PM +0200, Blue Swirl wrote: > > On 3/24/10, Michael S. Tsirkin <mst@redhat.com> wrote: > > > On Wed, Mar 24, 2010 at 10:05:10PM +0200, Blue Swirl wrote: > > > > On 3/24/10, Michael S. Tsirkin <mst@redhat.com> wrote: > > > > > On Tue, Mar 23, 2010 at 11:43:51PM +0200, Blue Swirl wrote: > > > > > > rtl8139.c, e1000.c: need to convert ldl/stl to cpu_physical_memory_read/write. > > > > > > > > > > > > > > > I don't see how it would help. These still get target_phys_addr_t which > > > > > is per-target. Further, a ton of devices do > > > > > cpu_register_physical_memory/qemu_register_coalesced_mmio. > > > > > These are also per target. > > > > > > > > I don't know what I was eating yesterday: there are no references to > > > > ldl/stl in either rtl8139 or e1000. In fact, the conversion is simple > > > > for the device itself, just add a property "be". The attached patch > > > > performs this part. > > > > > > > > But now there is a bigger problem, how to pass the property to the > > > > device. It's not fair to require the user to remember to set it. > > > > > > > > > I still don't fully understand how come e1000 cares about > > > target endianness. > > > > It shouldn't. Maybe the real fix is to remove the byte swapping. > > > Presumably it's there for a reason? > > > > > > > A simple solution would be to change all of cpu_XX functions to > > > > > get a 64 bit address. This is a lot of churn, if we do this > > > > > anyway we should also pass length to callbacks, this way > > > > > rwhandler will get very small or go away completely. > > > > > > > > It's not too much effort to keep the target_phys_addr_t type. > > > > > > > > > I don't understand - target_phys_addr_t is different for different > > > targets to we will need to recompile the code for each target. > > > What am I missing? > > > > target_phys_addr_t is 64 bit on a 64 bit host, on a 32 bit host it's > > size will be either 64 or 32 bits depending on the target. So the > > files are compiled once on 64 bit host, twice on 32 bit host if both > > 32 and 64 bits targets are selected. > > > How about just making it 64 bit unconditionally? > How much do we save by using a 32 bit address value? On a 32 bit host, probably a lot because of register pressure. And it's not too much effort to keep the target_phys_addr_t type logic.
On Wed, Mar 24, 2010 at 10:24:12PM +0200, Blue Swirl wrote: > On 3/24/10, Michael S. Tsirkin <mst@redhat.com> wrote: > > On Wed, Mar 24, 2010 at 10:05:10PM +0200, Blue Swirl wrote: > > > On 3/24/10, Michael S. Tsirkin <mst@redhat.com> wrote: > > > > On Tue, Mar 23, 2010 at 11:43:51PM +0200, Blue Swirl wrote: > > > > > rtl8139.c, e1000.c: need to convert ldl/stl to cpu_physical_memory_read/write. > > > > > > > > > > > > I don't see how it would help. These still get target_phys_addr_t which > > > > is per-target. Further, a ton of devices do > > > > cpu_register_physical_memory/qemu_register_coalesced_mmio. > > > > These are also per target. > > > > > > I don't know what I was eating yesterday: there are no references to > > > ldl/stl in either rtl8139 or e1000. In fact, the conversion is simple > > > for the device itself, just add a property "be". The attached patch > > > performs this part. > > > > > > But now there is a bigger problem, how to pass the property to the > > > device. It's not fair to require the user to remember to set it. > > > > > > I still don't fully understand how come e1000 cares about > > target endianness. > > It shouldn't. Maybe the real fix is to remove the byte swapping. > The real fix is actually to add a layer handling bus byte swapping depending on how bus are connected. Currently it only works because all big endian boards QEMU emulates need to byteswap bus access, and none of the little endian boards need to do that.
> But now there is a bigger problem, how to pass the property to the > device. It's not fair to require the user to remember to set it. It should not be a property of the device. All devices have a native endianness (for PCI this is little-endian), and the intermediate busses/interconnects should determine whether byteswapping occurs. Paul
On 03/24/2010 05:33 PM, Paul Brook wrote: >> But now there is a bigger problem, how to pass the property to the >> device. It's not fair to require the user to remember to set it. >> > It should not be a property of the device. All devices have a native > endianness (for PCI this is little-endian), and the intermediate > busses/interconnects should determine whether byteswapping occurs. > Right, the byte swapping needs to happen at the bus level which requires that the PCI regions use a different callback mechanism (and don't register directly with the cpu). Regards, Anthony Liguori > Paul > > >
> On 03/24/2010 05:33 PM, Paul Brook wrote: > >> But now there is a bigger problem, how to pass the property to the > >> device. It's not fair to require the user to remember to set it. > > > > It should not be a property of the device. All devices have a native > > endianness (for PCI this is little-endian), and the intermediate > > busses/interconnects should determine whether byteswapping occurs. > > Right, the byte swapping needs to happen at the bus level which requires > that the PCI regions use a different callback mechanism (and don't > register directly with the cpu). Not necessarily a different callback mechanism, but probably a different registration mechanism. Paul
On 03/24/2010 06:05 PM, Paul Brook wrote: >> On 03/24/2010 05:33 PM, Paul Brook wrote: >> >>>> But now there is a bigger problem, how to pass the property to the >>>> device. It's not fair to require the user to remember to set it. >>>> >>> It should not be a property of the device. All devices have a native >>> endianness (for PCI this is little-endian), and the intermediate >>> busses/interconnects should determine whether byteswapping occurs. >>> >> Right, the byte swapping needs to happen at the bus level which requires >> that the PCI regions use a different callback mechanism (and don't >> register directly with the cpu). >> > Not necessarily a different callback mechanism, but probably a different > registration mechanism. > Problem with the current scheme is that it's tied to target_phys_addr_t. It's not a target_phys_addr_t and cannot be used with functions that take target_phys_addr_ts. We can either introduce a generic address type or we can introduce bus specific addresses and have different callbacks. The later has better type safety and since this isn't an obvious issue to most developers, that's probably an important feature. Regards, Anthony Liguori > Paul >
On Wed, Mar 24, 2010 at 06:07:35PM -0500, Anthony Liguori wrote: > On 03/24/2010 06:05 PM, Paul Brook wrote: >>> On 03/24/2010 05:33 PM, Paul Brook wrote: >>> >>>>> But now there is a bigger problem, how to pass the property to the >>>>> device. It's not fair to require the user to remember to set it. >>>>> >>>> It should not be a property of the device. All devices have a native >>>> endianness (for PCI this is little-endian), and the intermediate >>>> busses/interconnects should determine whether byteswapping occurs. >>>> >>> Right, the byte swapping needs to happen at the bus level which requires >>> that the PCI regions use a different callback mechanism (and don't >>> register directly with the cpu). >>> >> Not necessarily a different callback mechanism, but probably a different >> registration mechanism. >> > > Problem with the current scheme is that it's tied to target_phys_addr_t. > It's not a target_phys_addr_t and cannot be used with functions that take > target_phys_addr_ts. > > We can either introduce a generic address type or we can introduce bus > specific addresses and have different callbacks. The later has better > type safety and since this isn't an obvious issue to most developers, > that's probably an important feature. > > Regards, > > Anthony Liguori > >> Paul >> I'd prefer a generic address type since this makes it easier to share code: for example virtio devices can use different busses, it's common for pci host bridges to have common code for i/o and memory, etc.
> I'd prefer a generic address type since this makes it easier to > share code: for example virtio devices can use different busses, This is exactly why virtio devices should not be accessing memory. They should be doing everything via a virtqueue, which can interface with the host bindings appropriately. Paul
From e0ab5cc41c68207be558ccb330f4fb83fba4ee6f Mon Sep 17 00:00:00 2001 From: Blue Swirl <blauwirbel@gmail.com> Date: Wed, 24 Mar 2010 19:54:05 +0000 Subject: [PATCH] Compile rtl8139 and e1000 only once WIP Signed-off-by: Blue Swirl <blauwirbel@gmail.com> --- Makefile.objs | 2 + Makefile.target | 4 -- hw/e1000.c | 108 ++++++++++++++++++++++++++++++++++++++++++------------ hw/rtl8139.c | 82 +++++++++++++++++++++++++++++++----------- 4 files changed, 147 insertions(+), 49 deletions(-) diff --git a/Makefile.objs b/Makefile.objs index 281f7a6..54895f8 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -155,6 +155,8 @@ hw-obj-y += msix.o hw-obj-y += ne2000.o hw-obj-y += eepro100.o hw-obj-y += pcnet.o +hw-obj-y += rtl8139.o +hw-obj-y += e1000.o hw-obj-$(CONFIG_SMC91C111) += smc91c111.o hw-obj-$(CONFIG_LAN9118) += lan9118.o diff --git a/Makefile.target b/Makefile.target index eb4d010..1a86fc4 100644 --- a/Makefile.target +++ b/Makefile.target @@ -176,10 +176,6 @@ QEMU_CFLAGS += $(VNC_SASL_CFLAGS) # xen backend driver support obj-$(CONFIG_XEN) += xen_machine_pv.o xen_domainbuild.o -# PCI network cards -obj-y += rtl8139.o -obj-y += e1000.o - # Hardware support obj-i386-y = ide/core.o obj-i386-y += pckbd.o dma.o diff --git a/hw/e1000.c b/hw/e1000.c index fd3059a..0f72db8 100644 --- a/hw/e1000.c +++ b/hw/e1000.c @@ -121,6 +121,7 @@ typedef struct E1000State_st { uint16_t reading; uint32_t old_eecd; } eecd_state; + uint32_t be; } E1000State; #define defreg(x) x = (E1000_##x>>2) @@ -825,14 +826,11 @@ static void (*macreg_writeops[])(E1000State *, int, uint32_t) = { enum { NWRITEOPS = ARRAY_SIZE(macreg_writeops) }; static void -e1000_mmio_writel(void *opaque, target_phys_addr_t addr, uint32_t val) +e1000_mmio_writel_le(void *opaque, target_phys_addr_t addr, uint32_t val) { E1000State *s = opaque; unsigned int index = (addr & 0x1ffff) >> 2; -#ifdef TARGET_WORDS_BIGENDIAN - val = bswap32(val); -#endif if (index < NWRITEOPS && macreg_writeops[index]) macreg_writeops[index](s, index, val); else if (index < NREADOPS && macreg_readops[index]) @@ -841,25 +839,47 @@ e1000_mmio_writel(void *opaque, target_phys_addr_t addr, uint32_t val) DBGOUT(UNKNOWN, "MMIO unknown write addr=0x%08x,val=0x%08x\n", index<<2, val); } +static void +e1000_mmio_writel_be(void *opaque, target_phys_addr_t addr, uint32_t val) +{ + val = bswap32(val); + e1000_mmio_writel_le(opaque, addr, val); +} static void -e1000_mmio_writew(void *opaque, target_phys_addr_t addr, uint32_t val) +e1000_mmio_writew_le(void *opaque, target_phys_addr_t addr, uint32_t val) { // emulate hw without byte enables: no RMW - e1000_mmio_writel(opaque, addr & ~3, - (val & 0xffff) << (8*(addr & 3))); + e1000_mmio_writel_le(opaque, addr & ~3, + (val & 0xffff) << (8*(addr & 3))); } static void -e1000_mmio_writeb(void *opaque, target_phys_addr_t addr, uint32_t val) +e1000_mmio_writew_be(void *opaque, target_phys_addr_t addr, uint32_t val) { // emulate hw without byte enables: no RMW - e1000_mmio_writel(opaque, addr & ~3, - (val & 0xff) << (8*(addr & 3))); + e1000_mmio_writel_be(opaque, addr & ~3, + (val & 0xffff) << (8*(addr & 3))); +} + +static void +e1000_mmio_writeb_be(void *opaque, target_phys_addr_t addr, uint32_t val) +{ + // emulate hw without byte enables: no RMW + e1000_mmio_writel_be(opaque, addr & ~3, + (val & 0xff) << (8*(addr & 3))); +} + +static void +e1000_mmio_writeb_le(void *opaque, target_phys_addr_t addr, uint32_t val) +{ + // emulate hw without byte enables: no RMW + e1000_mmio_writel_le(opaque, addr & ~3, + (val & 0xff) << (8*(addr & 3))); } static uint32_t -e1000_mmio_readl(void *opaque, target_phys_addr_t addr) +e1000_mmio_readl_le(void *opaque, target_phys_addr_t addr) { E1000State *s = opaque; unsigned int index = (addr & 0x1ffff) >> 2; @@ -867,9 +887,6 @@ e1000_mmio_readl(void *opaque, target_phys_addr_t addr) if (index < NREADOPS && macreg_readops[index]) { uint32_t val = macreg_readops[index](s, index); -#ifdef TARGET_WORDS_BIGENDIAN - val = bswap32(val); -#endif return val; } DBGOUT(UNKNOWN, "MMIO unknown read addr=0x%08x\n", index<<2); @@ -877,16 +894,38 @@ e1000_mmio_readl(void *opaque, target_phys_addr_t addr) } static uint32_t -e1000_mmio_readb(void *opaque, target_phys_addr_t addr) +e1000_mmio_readl_be(void *opaque, target_phys_addr_t addr) { - return ((e1000_mmio_readl(opaque, addr & ~3)) >> + uint32_t val = e1000_mmio_readl_le(opaque, addr); + val = bswap32(val); + return val; +} + +static uint32_t +e1000_mmio_readb_be(void *opaque, target_phys_addr_t addr) +{ + return ((e1000_mmio_readl_be(opaque, addr & ~3)) >> (8 * (addr & 3))) & 0xff; } static uint32_t -e1000_mmio_readw(void *opaque, target_phys_addr_t addr) +e1000_mmio_readb_le(void *opaque, target_phys_addr_t addr) +{ + return ((e1000_mmio_readl_le(opaque, addr & ~3)) >> + (8 * (addr & 3))) & 0xff; +} + +static uint32_t +e1000_mmio_readw_le(void *opaque, target_phys_addr_t addr) +{ + return ((e1000_mmio_readl_le(opaque, addr & ~3)) >> + (8 * (addr & 3))) & 0xffff; +} + +static uint32_t +e1000_mmio_readw_be(void *opaque, target_phys_addr_t addr) { - return ((e1000_mmio_readl(opaque, addr & ~3)) >> + return ((e1000_mmio_readl_be(opaque, addr & ~3)) >> (8 * (addr & 3))) & 0xffff; } @@ -1008,13 +1047,28 @@ static const uint32_t mac_reg_init[] = { }; /* PCI interface */ +static CPUWriteMemoryFunc * const e1000_mmio_write_be[] = { + e1000_mmio_writeb_be, + e1000_mmio_writew_be, + e1000_mmio_writel_be +}; -static CPUWriteMemoryFunc * const e1000_mmio_write[] = { - e1000_mmio_writeb, e1000_mmio_writew, e1000_mmio_writel +static CPUReadMemoryFunc * const e1000_mmio_read_be[] = { + e1000_mmio_readb_be, + e1000_mmio_readw_be, + e1000_mmio_readl_be }; -static CPUReadMemoryFunc * const e1000_mmio_read[] = { - e1000_mmio_readb, e1000_mmio_readw, e1000_mmio_readl +static CPUWriteMemoryFunc * const e1000_mmio_write_le[] = { + e1000_mmio_writeb_le, + e1000_mmio_writew_le, + e1000_mmio_writel_le +}; + +static CPUReadMemoryFunc * const e1000_mmio_read_le[] = { + e1000_mmio_readb_le, + e1000_mmio_readw_le, + e1000_mmio_readl_le }; static void @@ -1102,8 +1156,13 @@ static int pci_e1000_init(PCIDevice *pci_dev) /* TODO: RST# value should be 0 if programmable, PCI spec 6.2.4 */ pci_conf[PCI_INTERRUPT_PIN] = 1; // interrupt pin 0 - d->mmio_index = cpu_register_io_memory(e1000_mmio_read, - e1000_mmio_write, d); + if (d->be) { + d->mmio_index = cpu_register_io_memory(e1000_mmio_read_be, + e1000_mmio_write_be, d); + } else { + d->mmio_index = cpu_register_io_memory(e1000_mmio_read_le, + e1000_mmio_write_le, d); + } pci_register_bar((PCIDevice *)d, 0, PNPMMIO_SIZE, PCI_BASE_ADDRESS_SPACE_MEMORY, e1000_mmio_map); @@ -1146,6 +1205,7 @@ static PCIDeviceInfo e1000_info = { .romfile = "pxe-e1000.bin", .qdev.props = (Property[]) { DEFINE_NIC_PROPERTIES(E1000State, conf), + DEFINE_PROP_UINT32("be", E1000State, be, 0), DEFINE_PROP_END_OF_LIST(), } }; diff --git a/hw/rtl8139.c b/hw/rtl8139.c index 72e2242..ef5f1fd 100644 --- a/hw/rtl8139.c +++ b/hw/rtl8139.c @@ -493,7 +493,7 @@ typedef struct RTL8139State { /* PCI interrupt timer */ QEMUTimer *timer; int64_t TimerExpire; - + uint32_t be; } RTL8139State; static void rtl8139_set_next_tctr_time(RTL8139State *s, int64_t current_time); @@ -3123,19 +3123,29 @@ static void rtl8139_mmio_writeb(void *opaque, target_phys_addr_t addr, uint32_t rtl8139_io_writeb(opaque, addr & 0xFF, val); } -static void rtl8139_mmio_writew(void *opaque, target_phys_addr_t addr, uint32_t val) +static void rtl8139_mmio_writew_be(void *opaque, target_phys_addr_t addr, + uint32_t val) { -#ifdef TARGET_WORDS_BIGENDIAN val = bswap16(val); -#endif rtl8139_io_writew(opaque, addr & 0xFF, val); } -static void rtl8139_mmio_writel(void *opaque, target_phys_addr_t addr, uint32_t val) +static void rtl8139_mmio_writew_le(void *opaque, target_phys_addr_t addr, + uint32_t val) +{ + rtl8139_io_writew(opaque, addr & 0xFF, val); +} + +static void rtl8139_mmio_writel_be(void *opaque, target_phys_addr_t addr, + uint32_t val) { -#ifdef TARGET_WORDS_BIGENDIAN val = bswap32(val); -#endif + rtl8139_io_writel(opaque, addr & 0xFF, val); +} + +static void rtl8139_mmio_writel_le(void *opaque, target_phys_addr_t addr, + uint32_t val) +{ rtl8139_io_writel(opaque, addr & 0xFF, val); } @@ -3144,21 +3154,31 @@ static uint32_t rtl8139_mmio_readb(void *opaque, target_phys_addr_t addr) return rtl8139_io_readb(opaque, addr & 0xFF); } -static uint32_t rtl8139_mmio_readw(void *opaque, target_phys_addr_t addr) +static uint32_t rtl8139_mmio_readw_be(void *opaque, target_phys_addr_t addr) { uint32_t val = rtl8139_io_readw(opaque, addr & 0xFF); -#ifdef TARGET_WORDS_BIGENDIAN val = bswap16(val); -#endif return val; } -static uint32_t rtl8139_mmio_readl(void *opaque, target_phys_addr_t addr) +static uint32_t rtl8139_mmio_readw_le(void *opaque, target_phys_addr_t addr) +{ + uint32_t val = rtl8139_io_readw(opaque, addr & 0xFF); + + return val; +} + +static uint32_t rtl8139_mmio_readl_be(void *opaque, target_phys_addr_t addr) { uint32_t val = rtl8139_io_readl(opaque, addr & 0xFF); -#ifdef TARGET_WORDS_BIGENDIAN val = bswap32(val); -#endif + return val; +} + +static uint32_t rtl8139_mmio_readl_le(void *opaque, target_phys_addr_t addr) +{ + uint32_t val = rtl8139_io_readl(opaque, addr & 0xFF); + return val; } @@ -3292,16 +3312,28 @@ static void rtl8139_ioport_map(PCIDevice *pci_dev, int region_num, register_ioport_read( addr, 0x100, 4, rtl8139_ioport_readl, s); } -static CPUReadMemoryFunc * const rtl8139_mmio_read[3] = { +static CPUReadMemoryFunc * const rtl8139_mmio_read_be[3] = { rtl8139_mmio_readb, - rtl8139_mmio_readw, - rtl8139_mmio_readl, + rtl8139_mmio_readw_be, + rtl8139_mmio_readl_be, }; -static CPUWriteMemoryFunc * const rtl8139_mmio_write[3] = { +static CPUWriteMemoryFunc * const rtl8139_mmio_write_be[3] = { rtl8139_mmio_writeb, - rtl8139_mmio_writew, - rtl8139_mmio_writel, + rtl8139_mmio_writew_be, + rtl8139_mmio_writel_be, +}; + +static CPUReadMemoryFunc * const rtl8139_mmio_read_le[3] = { + rtl8139_mmio_readb, + rtl8139_mmio_readw_le, + rtl8139_mmio_readl_le, +}; + +static CPUWriteMemoryFunc * const rtl8139_mmio_write_le[3] = { + rtl8139_mmio_writeb, + rtl8139_mmio_writew_le, + rtl8139_mmio_writel_le, }; static void rtl8139_timer(void *opaque) @@ -3369,8 +3401,15 @@ static int pci_rtl8139_init(PCIDevice *dev) pci_conf[PCI_CAPABILITY_LIST] = 0xdc; /* I/O handler for memory-mapped I/O */ - s->rtl8139_mmio_io_addr = - cpu_register_io_memory(rtl8139_mmio_read, rtl8139_mmio_write, s); + if (s->be) { + s->rtl8139_mmio_io_addr = + cpu_register_io_memory(rtl8139_mmio_read_be, rtl8139_mmio_write_be, + s); + } else { + s->rtl8139_mmio_io_addr = + cpu_register_io_memory(rtl8139_mmio_read_le, rtl8139_mmio_write_le, + s); + } pci_register_bar(&s->dev, 0, 0x100, PCI_BASE_ADDRESS_SPACE_IO, rtl8139_ioport_map); @@ -3404,6 +3443,7 @@ static PCIDeviceInfo rtl8139_info = { .romfile = "pxe-rtl8139.bin", .qdev.props = (Property[]) { DEFINE_NIC_PROPERTIES(RTL8139State, conf), + DEFINE_PROP_UINT32("be", RTL8139State, be, 0), DEFINE_PROP_END_OF_LIST(), } }; -- 1.5.6.5