[Qemu-devel] PATCH: Ensure RTL 8139 MMIO regions are 4k apart

[Qemu-devel] PATCH: Ensure RTL 8139 MMIO regions are 4k apart

[Daniel P. Berrange]

[-- Attachment #1: Type: text/plain, Size: 3006 bytes --]

The current QEMU code for the RTL-8193 network device has some issues if 
there is more than one device activated in a guest. Specifically, even
if you specify difference MAC addresses, inside the guest all NICs end
up seeing the same MAC - the MAC of the last NIC.

Full details are recorded in this bug report

    https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=247641

But for brevity, the situation is thus...

Launchint a guest with two NICs, giving different MAC address

/usr/bin/qemu -hda /root/test.img \
   -net nic,macaddr=00:16:3e:58:0d:35,vlan=0,model=rtl8139 -net tap,fd=14,vlan=0 \
   -net nic,macaddr=00:16:3e:63:21:a5,vlan=1,model=rtl8139 -net tap,fd=16,vlan=1

Running Fedora 7 in the guest when the 8139cp driver is loaded it shows

 eth0: RTL-8139C+ at 0xffffc2000000c000, 00:16:3e:63:21:a5, IRQ 11
 eth1: RTL-8139C+ at 0xffffc2000000e100, 00:16:3e:63:21:a5, IRQ 9

Notice the MAC's are the same. Poking at QEMU internal data structures
with GDB shows that QEMU has the correct MAC addresses everywhere. After
a looking at the Linux 8139cp driver it gets the MAC adress by reading
the EEPROM on the nic.

Further debugging revealed that all EEPROM read/writes in QEMU were being
directed to the last configured NIC, regardless of which NIC the guest was
trying to access.

The code which converts from a MMIO address back to the RTL8139State *
data is in softmmu-template.h, in the first 'glue' method. Specifically

   index = (tlb_adr >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1)

Since IO_MEM_NB_ENTRIES is defined in terms of TARGET_PAGE_BITS, this
conversion will only work if each NIC's MMIO region is mapped at least
1 page apart. Unfortunately the RTL 8139 NIC is mapping 256 byte regions.

The hw/rtl8139.c code is passing  0x100 in as the size parameter for the
cpu_register_physical_memory call, despite the fact that the API contract
says size has to be a multiple of page size.

The attached patch fixes the rtl8139.c to request 0x1000 as the size, likewise
when registering the PCI io regions. I'm not sure if the hardware specs require
than the IO regions with RTL 8139 are 256 bytes in size. Increasing them to
4k didn't seem to cause any ill effects - with this patch applied the guest OS
at least sees the correct EEPROM per NIC.  May be an alternate approach would
be to allow  cpu_register_physical_memory to take a size < 1 page, but have
it align the resulting physical address to the next highest page boundary

NB, there's plenty of other places in the code which are passing in a size
parameter to cpu_register_physical_memory which are smaller than 1 page. I
have not looked at these to see whether there's any ill effects.

Regards,
Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

[-- Attachment #2: kvm-rtl8139-mmio-regions.patch --]
[-- Type: text/plain, Size: 1171 bytes --]

diff -rup kvm-24/qemu/hw/rtl8139.c kvm-24.new/qemu/hw/rtl8139.c
--- kvm-24/qemu/hw/rtl8139.c	2007-05-08 04:44:27.000000000 -0400
+++ kvm-24.new/qemu/hw/rtl8139.c	2007-07-16 11:12:00.000000000 -0400
@@ -3325,7 +3325,7 @@ static void rtl8139_mmio_map(PCIDevice *
     PCIRTL8139State *d = (PCIRTL8139State *)pci_dev;
     RTL8139State *s = &d->rtl8139;
 
-    cpu_register_physical_memory(addr + 0, 0x100, s->rtl8139_mmio_io_addr);
+    cpu_register_physical_memory(addr + 0, 0x1000, s->rtl8139_mmio_io_addr);
 }
 
 static void rtl8139_ioport_map(PCIDevice *pci_dev, int region_num, 
@@ -3438,10 +3438,10 @@ void pci_rtl8139_init(PCIBus *bus, NICIn
     s->rtl8139_mmio_io_addr =
     cpu_register_io_memory(0, rtl8139_mmio_read, rtl8139_mmio_write, s);
 
-    pci_register_io_region(&d->dev, 0, 0x100, 
+    pci_register_io_region(&d->dev, 0, 0x1000, 
                            PCI_ADDRESS_SPACE_IO,  rtl8139_ioport_map);
 
-    pci_register_io_region(&d->dev, 1, 0x100, 
+    pci_register_io_region(&d->dev, 1, 0x1000, 
                            PCI_ADDRESS_SPACE_MEM, rtl8139_mmio_map);
 
     s->irq = 16; /* PCI interrupt */
Only in kvm-24.new/qemu/hw: rtl8139.c~

Re: [Qemu-devel] PATCH: Ensure RTL 8139 MMIO regions are 4k apart

[Paul Brook]

> The hw/rtl8139.c code is passing  0x100 in as the size parameter for the
> cpu_register_physical_memory call, despite the fact that the API contract
> says size has to be a multiple of page size.

Have you tried recent CVS? This should not be necessary.

Paul

Re: [Qemu-devel] PATCH: Ensure RTL 8139 MMIO regions are 4k apart

[Daniel P. Berrange]

On Mon, Jul 16, 2007 at 06:07:18PM +0100, Paul Brook wrote:
> > The hw/rtl8139.c code is passing  0x100 in as the size parameter for the
> > cpu_register_physical_memory call, despite the fact that the API contract
> > says size has to be a multiple of page size.
> 
> Have you tried recent CVS? This should not be necessary.

I thought I had - but my checkout was synced to the 0.9.0 release :-( Re-tested
with latest HEAD and it works OK without needing this patch. Sorry for the 
noise.

Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=|