On Wed, Mar 24, 2010 at 10:05:10PM +0200, Blue Swirl wrote: > On 3/24/10, Michael S. Tsirkin <m...@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 <blauwir...@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 <blauwir...@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 >