config, x86: add ethernet support for x86 fullsystem

Information
Submitter:	Jiuyue Ma
Repository:	gem5
Branch:
Bugs:
Depends On:	~~2300~~, ~~2314~~, ~~2315~~
Reviewers
Groups:	Default
People:

Description

config, x86: add ethernet support for x86 fullsystem

This patch add a IGbE_e1000 ethernet device to x86 fs system, it does
the followings:
1) add IGbE_e1000 to x86_sys.pc.ethernet,
2) connect x86_sys.pc.ethernet.pio/config/dma to x86_sys.iobus,
3) add interrupt assignment for x86_sys.pc.ethernet in MP table.

Testing Done

Issue Summary

Description	From	Last Updated	Status
It would be better if a suitable range was added to the bridge.	Andreas Hansson	June 15, 2014, 6:04 p.m.	Resolved
I do not understand why this is not covered by the last statement, i.e. the Pci config space until the ...	Andreas Hansson	June 17, 2014, 5:58 p.m.	Dropped
It would be good to add a line describing where the values come from.	Andreas Hansson	June 17, 2014, 6:43 p.m.	Resolved
Since the bus order is important, I'd suggest copying the statement around this from the patch summary and make sure ...	Andreas Hansson	June 17, 2014, 6:43 p.m.	Resolved
I might be wrong here, but I think the InterruptLine is actually the interrupt line as seen by the APIC. ...	Andreas Sandberg	July 16, 2014, 9:36 p.m.	Resolved

configs/common/FSConfig.py (Diff revision 1)

It would be better if a suitable range was added to the bridge.

Show all issues

Jiuyue Ma June 13, 2014, 8:58 p.m. (June 13, 2014, 8:58 p.m.)

I had tried to deal with this issue, but it seems a little bit hard.
To do this, the iobus must propagate address range change to bridge. But at the bridge side, it can't distinguish whether a addr_range was from iobridge/iocache or iodevice.
We should only add addr_range from iodevice to bridge's slave port, because addr_range from iobridge was overlapped with mem_ctrls.
Otherwise bridge and mem_ctrl will conflict at membus.
I've no idea how to deal with this... T_T

Change Summary:

connect bridge.slave to membus.master instead of membus.default
add [mem_size-1(or 3GB for >4GB memory), 0xFFFFFFFF] to bridge's ranges for kernel configured pci device memory

Description:

		config: add ethernet support for x86 fullsystem

		This patch add a IGbE_e1000 ethernet device to x86 fs system. To make it work properly,
		following changes were also made to FSConfig.py:

~		connect 'default' port of x86_sys.membus to io bridge instead of badaddr_responder, access to pci address space will pass though membus to iobridge
	~	add [mem_size-1(or 3GB for >4GB memory), 0xFFFFFFFF] to bridge's ranges for kernel configured pci device memory, access to pci address space will pass though membus to bridge
		add IGbE_e1000 to x86_sys.pc.ethernet
		connect x86_sys.pc.ethernet.pio/config/dma to x86_sys.iobus
		swap bus_id of ISA/PCI in X86 IntelMPTable In gem5 Pc::calcPciConfigAddr(), it required "assert(bus==0)", but linux kernel cannot config ethernet device connected to ISA bus, so we swap bus_id of ISA/PCI

Depends On:

-	~~2300 - x86: add LongModeAddressSize function to cpuid~~

Diff:

Revision 2 (+22 -14)

Show changes

configs/common/FSConfig.py

Change Summary:

add missing depends on #2300

Depends On:

+	~~2300 - x86: add LongModeAddressSize function to cpuid~~

configs/common/FSConfig.py (Diff revision 2)

I do not understand why this is not covered by the last statement, i.e. the Pci config space until the end of 64-bit space.

Show all issues

Jiuyue Ma June 17, 2014, 5:58 p.m. (June 17, 2014, 5:58 p.m.)

"[pci_config_address_space_base, Addr.max]" was the PCI config space, which only contains PCI config data (e.g. BAR0-5, VendorID, ...)

linux kernel config PCI BARx when probe device, remap ioport/iomem of device to host's address space (e.g. in my 32MB memory config, ethernet device's iomem was mapped to "10000000-1001ffff : 0000:00:02.0")
the BARx was 32-bit length and must be less than 4GB which is not covered by [pci_config_address_space_base, Addr.max]

p.s. this patch will not work when pci device support 64-bit memory BAR in the future, because kernel may config PCI BARx above 4GB

configs/common/FSConfig.py (Diff revision 2)

It would be good to add a line describing where the values come from.

Show all issues

Jiuyue Ma June 17, 2014, 6:43 p.m. (June 17, 2014, 6:43 p.m.)

Change InterruptLine from IRQ13(reserved for co-processor/FPU) to IRQ10(open interrupt/available or SCSI or NIC)
reference http://en.wikipedia.org/wiki/Interrupt_request#x86_IRQs

configs/common/FSConfig.py (Diff revision 2)

Since the bus order is important, I'd suggest copying the statement around this from the patch summary and make sure it also appears in comments in the code.

Show all issues

Change Summary:

change IRQ of ethernet from IRQ13 to IRQ10
add comment for IGbE config
add comment for bus_id config of X86IntelMPTable

Diff:

Revision 3 (+29 -14)

Show changes

configs/common/FSConfig.py

Could you split this into two (or potentially three) different patches?

The PCI/ISA bus ID fixes look fine and should definitely go upstream ASAP. As far as I'm concerned that particular part of the patch can be submitted separately right away as it is a small bug fixes that is limited to a few lines.

I'm not so sure about the bridge ranges though. The PCI specification seems to allow devices to be mapped to pretty much any region of the memory space, so we can probably not do what you're doing currently to determine the range. Unfortunately, I can't think of a good solution off the top of my head. I'll discuss it with some colleagues and get back to you.

Jiuyue Ma July 16, 2014, 3:01 a.m. (July 16, 2014, 3:01 a.m.)

Split into three patches may be better:
1) PCI/ISA bus ID fixes
2) add/connect ethernet device
3) bridge range fixes
I will do this split latter tomorrow.

About the bridge range, I have tried to modify recvRangeChange() interface of Bridge: change bridge's slave port range when its master side changed.
I think this should be a better solution than current one. But I failed to deal with the address conflict of mem_ctrls and iobridge/iocache in membus. T_T

configs/common/FSConfig.py (Diff revision 3)

I might be wrong here, but I think the InterruptLine is actually the interrupt line as seen by the APIC. It doesn't have anything to do with the actual CPU interrupt since the APIC sorts out the routing. If this is the case, get rid of that part of the comment to avoid confusion.

Show all issues

Jiuyue Ma July 16, 2014, 2:45 a.m. (July 16, 2014, 2:45 a.m.)

Yes, you are right. The InterruptLine is the interrupt line as seen by the APIC. I will remove these comments.

An interesting thing is when I change this value in python config, kernel also see the same change. I have no idea of why this happened, maybe kernel init irq routing table using hardware probed InterruptLine when system startup? I only found irq assign in generic pci driver "linux-2.6.28.4/drivers/pci/probe.c:664", but it is not used by x86 arch.

 660 /*
 661  * Read interrupt line and base address registers.
 662  * The architecture-dependent code can tweak these, of course.
 663  */
 664 static void pci_read_irq(struct pci_dev *dev)
 665 {
 666         unsigned char irq;
 667 
 668         pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &irq);
 669         dev->pin = irq;
 670         if (irq)
 671                 pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &irq);
 672         dev->irq = irq;
 673 }

Change Summary:

Remove comment for InterruptLine assign.
Split into three patches:
- 2314: PCI/ISA bus ID fixes
- 2315: bridge range fixes
- 2310(this one): add/connect ethernet device

Description:

		config: add ethernet support for x86 fullsystem

~		This patch add a IGbE_e1000 ethernet device to x86 fs system. To make it work properly,
	~	This patch add a IGbE_e1000 ethernet device to x86 fs system.
-		following changes were also made to FSConfig.py:
-
-		add [mem_size-1(or 3GB for >4GB memory), 0xFFFFFFFF] to bridge's ranges for kernel configured pci device memory, access to pci address space will pass though membus to bridge
-		add IGbE_e1000 to x86_sys.pc.ethernet
-		connect x86_sys.pc.ethernet.pio/config/dma to x86_sys.iobus
-		swap bus_id of ISA/PCI in X86 IntelMPTable In gem5 Pc::calcPciConfigAddr(), it required "assert(bus==0)", but linux kernel cannot config ethernet device connected to ISA bus, so we swap bus_id of ISA/PCI

Depends On:

+	~~2314 - config, x86: swap bus_id of ISA/PCI in X86 IntelMPTable~~
+	~~2315 - config, x86: Ensure that PCI devs get bridged to the memory bus~~

Diff:

Revision 4 (+9)

Show changes

configs/common/FSConfig.py

I'm sorry I didn't spot this earlier, but it seems like you forgot to include the ethernet device's interrupt in the MP table. See the interrupt declaration for pci_dev4_inta for an example. Without that entry, Linux won't be able to setup interrupt routing properly.

Jiuyue Ma July 17, 2014, 2:55 a.m. (July 17, 2014, 2:55 a.m.)

We don't need anything in MP table, kernel can configure PCI device's interrupt itself :)
Maybe only ISA device need a MP table entry?

After apply 2314/2315/2310 and 2300(needed by 2.6.28.4; 2.6.22.9 did not care this),
I can boot system and discover&configure ethernet device successfully.

----------------------- boot log ----------------------------
Intel(R) PRO/1000 Network Driver - version 7.3.20-k2Copyright (c) 1999-2006 Intel Corporation.
PCI: Enabling device 0000:00:02.0 (0000 -> 0002)
e1000: 0000:00:02.0: e1000_probe: (PCI:33MHz:32-bit) 00:90:00:00:00:01
e1000: eth0: e1000_probe: Intel(R) PRO/1000 Network Connection

----------------------- config ----------------------------
loading script...
Script from M5 readfile is empty, starting bash shell...
(none) / # ifconfig eth0 192.168.0.1
ADDRCONF(NETDEV_UP): eth0: link is not ready
(none) / # e1000: eth0: e1000_watchdog: NIC Link is Up 1000 Mbps Full Duplex, Flow Control: None
ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
ifconfig lo 127.0.0.1
(none) / # ping 192.168.0.1
PING 192.168.0.1 (192.168.0.1) 56(84) bytes of data.
64 bytes from 192.168.0.1: icmp_seq=1 ttl=64 time=0.008 ms

--- 192.168.0.1 ping statistics ---
1 packets transmitted, 1 received, 0% packet loss, time 0ms
rtt min/avg/max/mdev = 0.008/0.008/0.008/0.000 ms

Andreas Sandberg July 17, 2014, 3:25 a.m. (July 17, 2014, 3:25 a.m.)

It probably depends on the kernel version. I tested with 3.4.94 and got a warning that the MP table was broken because it didn't find the interrupt routing information for the ethernet device. I think the warning even stated that the kernel was unable to figure out the interrupt.

Jiuyue Ma July 17, 2014, 5:31 p.m. (July 17, 2014, 5:31 p.m.)
```
fixed :)
```

There is still something to do with x86 ethernet support:
When using atomic mode, self-ping never get any output after a single line of output;
simple-timing mode will get 2-lines of output.
I can not figure out what happened. Maybe someone can help to solve this problem.

Change Summary:

add interrupt assignment for ethernet in MP table
change summary line to "config, x86"

Summary:

-	config: add ethernet support for x86 fullsystem
+	config, x86: add ethernet support for x86 fullsystem

Description:

~		config: add ethernet support for x86 fullsystem
	~	config, x86: add ethernet support for x86 fullsystem

~		This patch add a IGbE_e1000 ethernet device to x86 fs system.
	~	This patch add a IGbE_e1000 ethernet device to x86 fs system, it does
	+	the followings:
	+	1) add IGbE_e1000 to x86_sys.pc.ethernet,
	+	2) connect x86_sys.pc.ethernet.pio/config/dma to x86_sys.iobus,
	+	3) add interrupt assignment for x86_sys.pc.ethernet in MP table.

Diff:

Revision 5 (+19)

Show changes

configs/common/FSConfig.py

LGTM. Thanks for fixing this!

Mohammad Alian Sept. 2, 2014, 9:12 a.m. (Sept. 2, 2014, 9:12 a.m.)

Hi,
Have you ever tried to boot system or use Ethernet device with O3 cpu model? With O3 cpu I get this error:

gem5.opt: build/X86/dev/i8254xGBe.cc:182: virtual Tick IGbE::read(PacketPtr): Assertion `pkt->getSize() == 4' failed.
Program aborted at tick 131516086224500
Aborted

The problem is that 'pkt-getSize()' returns 64 with O3 cpu model ...
Any idea how to fix it?

Thanks,
Mohammad

You have a pending review.

Review Board 2.0.15

Screenshots

Files

Issue Summary

Change Summary:

Description:

Depends On:

Diff:

Change Summary:

Depends On:

Change Summary:

Diff:

Change Summary:

Description:

Depends On:

Diff:

Change Summary:

Summary:

Description:

Diff: