On 11/24/10 16:50, Anthony Liguori wrote:
> On 11/24/2010 09:40 AM, Gleb Natapov wrote:
>> On Wed, Nov 24, 2010 at 08:41:26AM -0600, Anthony Liguori wrote:
>>> In real hardware, the i8042 (keyboard controller) is actually
>>> implemented in the PIIX3 which is a chip that is part of the i440fx.
>>> The i440fx acts as both the memory controller and as the PCI Host
>>> controller. So you get something that looks like:
>>>
>>> class PIIX3 : public PCIDevice
>>> {
>>> private:
>>> I8042 i8042;
>>> RTC rtc;
>>> // ...
>>> };
>>>
>>>
>> The fact that in physical implementation they sit in the same silicon
>> does not mean that logically they belong to the same class. PIIX3
>> is ISA bridge. It doesn't mean it owns devices on the ISA bus it
>> provides. The information that you are trying to convey here belongs to
>> configuration file.
>
> Why would we specify a PIIX3 device based on a configuration file?
> There is only one PIIX3 device in the world. I don't see a lot of need
> to create arbitrary types of devices.
Well the problem here is that the i8042 is in the i440fx.c file, it
shouldn't be there in the first place. The gluing together things in
silicon is really just a way to shorten the wires and make it easier,
they are still separate devices and as long as the i8042 requires ISA
access, and to be treated like an ISA device, we should glue it onto the
virtual ISA bus within QEMU.
What you did above is making the exact same mistake as is done with the
current i440fx.c code.
> I think it's a lot of abstraction for very little gain and leads to
> bizarreness like treating any platform device as an ISA device.
>
> An RTC is *not* an ISA device. It may sit *behind* an ISA bus because
> the SuperIO chip happens to be on the ISA bus. But on modern systems,
> the ISA bus has disappeared in favor of the LPC but that doesn't
> fundamentally change what the RTC is.
>
> The problem with what you describe is that there are far fewer devices
> that actually sit on busses than what QEMU tries to model today.
A PCI device is mapped on a PCI bus as a PCI function etc. If the device
is hard wired to a fixed address, or worse a fixed IO port, then it is
an ISA device.
>>> So whereas we have this very complicate machine create function that
>>> attempts to create and composite all of these devices after the
>>> fact, when written in C++, partially due to good design, but
>>> partially due to the fact that the languages forces you to think a
>>> certain way, you get a tremendous simplification.
That is clearly dependent on the eyes of the person who looks at it.
>> That is if you are using code as data. With other design you actually
>> read I440FX configuration from file and build it from smaller parts.
>> You see C++ doesn't stop us from arguing what design is correct.
>>
>
> That's fair. I think 90% of what we need is better design. I think a
> better language only gives us 10%.
Well the problem is the 10% you are talking about is another 30% loss
because the code is now practically unreadable, plus you open up the can
of worms that people will start using some of the totally broken
features of C++. Sure you can try hard to make sure they don't sneak in,
but down the line they will, and at that point the codebase is totally
ruined.
Avi's unittest code is a perfect example of code that is unreadable for
a C programmer. Or to quote a smart man 'the code is clear as perl!'.
So you have now lost or at least crippled half your developer base,
making it way harder for them to contribute something useful.
This is a big step in the wrong direction :(
Jes
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
