IDE Disk: Bring it inline with QEMU

Information
Submitter:	Nilay Vaish
Repository:	gem5
Branch:	default
Bugs:
Depends On:
Reviewers
Groups:	Default
People:	ali, gblack, nate, stever

Description

IDE Disk: Bring it inline with QEMU
I have made some changes to the IDE Disk's ports so that it is inline with
QEMU. I am able to boot Linux 2.6.28.4 for x86 with these changes.

Testing Done

I'm sorry if I haven't been paying attention, but what do these changes have to do with QEMU?

Nilay Vaish Oct. 11, 2011, 12:28 a.m. (Oct. 11, 2011, 12:28 a.m.)

I should have been more elaborate. The io ports assigned to Ide Disk are
different from the those assigned in QEMU. Specifically QEMU assigns 
IDE BAR 1 to be 3f6 instead of 3f4 as it is in gem5 right now. The size
is 1 in place of 3.

I would expect these ports to be same for both gem5 and QEMU.

Nathan Binkert Oct. 11, 2011, 12:47 a.m. (Oct. 11, 2011, 12:47 a.m.)

I would agree that these should be the same, though the question is, why did you do this?  Was there a bug?  I assume that linux didn't work and this fixed it, but your commit message wasn't exactly clear (it may also mean that you simply tested it with that kernel version).  Finally, you added a dma controller.  What was that for?  Overall, the change seems fine and should probably go in, just a bit confused as to what it is for.

Nilay Vaish Oct. 11, 2011, 9:28 a.m. (Oct. 11, 2011, 9:28 a.m.)

I am trying to get a checkpoint obtained from QEMU based virtual
machine to work with gem5. Since it is not working out right now,
I am trying to figure out where the differences are. This is one
of the differences I noticed.

Linux kernel 2.6.28.4 works with current gem5, so I am not trying
to fix any bug as such. But as you pointed out that the ports used
in QEMU and gem5 should be same and this is exactly what I am aiming
for. The DMA controller exists in QEMU, that's why I added it.

Nathan Binkert Oct. 11, 2011, 9:38 a.m. (Oct. 11, 2011, 9:38 a.m.)

All sounds good.  These details should go in the commit message.

Ali Saidi Oct. 11, 2011, 12:07 p.m. (Oct. 11, 2011, 12:07 p.m.)

Before this goes in it needs to be tested with alpha and arm. Also, looking at the panics you removed, it seems like they were there because some functionality isn't imolemnented. Just removing them doesn't implement it. Finally, to really test this you probably need to do more than just boot Linux. You need to make sure that some writes hit the disk too.

Nilay Vaish Oct. 11, 2011, 2:51 p.m. (Oct. 11, 2011, 2:51 p.m.)

I agree that both alpha and arm should also work correctly. That's
something I am skeptical about and that's why I had mentioned x86
in my original description. I think QEMU works for alpha and arm,
so things should work out. I will try booting Linux kernel for
these architectures.

As far as disk writes are concerned, Linux kernel does write some
logs when the kernel boots. I did not check but I am expecting that
those would have happened correctly.

Gabe Black Oct. 11, 2011, 5:01 p.m. (Oct. 11, 2011, 5:01 p.m.)

I think what Ali meant was that we have to make sure this doesn't affect ARM and Alpha working properly, independent of if we can restore QEMU checkpoints to them.

Generally speaking it's probably handy to be able to use QEMU checkpoints, but I don't want us to be stuck doing things a certain way because QEMU did it that way. In this case it might be fine because this is probably just standard PC stuff, but I'd like a chance to look it over carefully. In cases where the config isn't fixed by how the architecture is defined, it may make sense to have a separate configuration that's specifically set up to be compatible with QEMU. I'll try to look into this properly this weekend.

Nilay Vaish Oct. 12, 2011, 8:45 a.m. (Oct. 12, 2011, 8:45 a.m.)

I compiled and ran ALPHA_FS and ARM_FS using fs.py. In both cases
the system booted and I was able to write to and read from a file.
Do you think this is enough evidence that changes are working
correctly?

src/dev/ide_ctrl.cc (Diff revision 1)

Does leaving this line out cause something not to work? I see that it's used for the primary controller so this makes them symmetrical. I don't remember what ctrlOffset is for so I don't know for sure if they *should* be symmetrical. If this fixed a failure of some kind then they probably should be.

src/dev/ide_disk.cc (Diff revision 1)

Why are you removing these panics and the constants they depend on? If they were going off before, then there's something that may need to be fixed. If they started going off when you changed the BARs below, then they weren't wrong at all, they just needed to be updated. But as I say below, I'm not convinced that the BARs should be changed.

src/dev/x86/SouthBridge.py (Diff revision 1)

I don't think these changes to the BARs are correct, and I don't see why this would matter when restoring a checkpoint.

Nilay Vaish Oct. 16, 2011, 2:21 p.m. (Oct. 16, 2011, 2:21 p.m.)

You are right. I checked the Linux kernel today. It seems to use
both 0x3f4 and 0x3f6. Hence, I will not be committing this patch.

src/dev/ide_ctrl.cc (Diff revision 1)

I'm fine with this, we should have been doing it before, it was just an oversight.

Nilay Vaish Oct. 17, 2011, 4:51 p.m. (Oct. 17, 2011, 4:51 p.m.)
```
I'll commit this line of code.
```

You have a pending review.

Review Board 2.0.15

This change has been discarded.

Screenshots

Files