Re: [PATCH kvm-unit-tests 2/4] Introduce a C++ wrapper for the kvm APIs

[Gleb Natapov <gleb@redhat.com>]

On Wed, Nov 24, 2010 at 09:50:35AM -0600, 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:
> >>On 11/24/2010 06:59 AM, Alexander Graf wrote:
> >>>On 24.11.2010, at 11:52, Avi Kivity wrote:
> >>>
> >>>>Introduce exception-safe objects for calling system, vm, and vcpu ioctls.
> >>>>
> >>>>Signed-off-by: Avi Kivity<avi@redhat.com>
> >>>FWIW, I still disagree with C++ and believe this code to be hardly readable.
> >>There's a general prettiness that well written C++ code will have
> >>over C when there's heavy object modelling.  This can be subjective
> >>but for me, it's fairly significant.
> >>
> >>The fact that objects are easily created on the stack and on the
> >>heap is also pretty significant.  When considering device models, we
> >>struggle today with device composition.
> >>
> >>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.
> 
Why deny this flexibility from those who need it for modelling
different HW? Besides, as I said, PIIX3 is ISA bridge and this
is what class should implement. We have fw_cfg on ISA bus too
which does not exits on real HW and we may want to have other
devices. We should be able to add them without changing PIIX3
class.

> The real world hierarchy matters when you're trying to model I/O dispatch.
> 
Or build device path. Any time we do something not as "real HW" we
regret it later.

> >  Here I go factory design pattern.
> 
> 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.
> 
Why?

> 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.
Agree. This is a device sitting on the ISA bus on a PC, but it can
sit on other buses too. And it happily does so on other architectures.

> 
> 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.
All devices sit on some buses.

> 
> >>class I440FX : public PCIHostController
> >>{
> >>    I440FX(void) {
> >>         this->slots[1].plug(&this->piix3); // piix3 is always in slot 1
> >>    }
> >>
> >>private:
> >>    Plug<PCIDevice *>  slots[32]; // slot 0 is the PMC
> >>    PIIX3 piix3;
> >>};
> >>
> >>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.
> >>
> >Forcing to think you in certain way does not make you suddenly make good
> >design decisions (if only programming was so easy). But it makes it even
> >harder to fix bad decision since suddenly all you design depends on it.
> >
> >>A proper C++ device model turns a vast majority of our device
> >>creation complexity into a single new I440FX.  Then it's just a
> >>matter of instantiating and plugging the appropriate set of PCI
> >>devices.
> >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%.
> 
> >>Another area that C++ shines is safety.  C++ enables you to inject
> >>safe versions of things that you really can't do in C.  For
> >>instance, the PIT has three channels but the mask to select a
> >>channel is two bits.  There was a kernel exploit that found a way to
> >>trick selection of a forth channel because of a missing check.
> >>
> >>In C++, you can convert:
> >>
> >>PITChannel channnels[3];
> >>
> >>Into:
> >>
> >>Array<PITChannel, 3>  channels;
> >>
> >Any sane modern language gives you that. Why C++?
> 
> Because I don't think we can implement a reasonable device model
> using a garbage collected language.  Garbage collection introduces
> non-determinism and in QEMU we need to ensure that when we're
> running in a VCPU context, we exit back to the guest as quickly as
> possible.
> 
IIRC there are garbage collected languages that allow you to disable garbage
collection for certain part of the code. Garbage collection can be done
while vcpu executes guest code and in proper implementation vcpu thread
should execute device model code for a very brief time and do not sleep
there. All this makes me think that garbage collection shouldn't be a
big issue for kvm userspace.
 
--
			Gleb.