qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Anthony Liguori <anthony@codemonkey.ws>
To: Markus Armbruster <armbru@redhat.com>
Cc: Blue Swirl <blauwirbel@gmail.com>,
	"Liu >> \"Liu, Jinsong\"" <jinsong.liu@intel.com>,
	Paul Brook <paul@codesourcery.com>, Avi Kivity <avi@redhat.com>,
	qemu-devel <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [PATCH v2 0/7] APIC/IOAPIC cleanup
Date: Fri, 20 Aug 2010 13:38:47 -0500	[thread overview]
Message-ID: <4C6ECBB7.7060608@codemonkey.ws> (raw)
In-Reply-To: <m362z55ibn.fsf@blackfin.pond.sub.org>

On 08/20/2010 12:01 PM, Markus Armbruster wrote:
> Anthony Liguori<anthony@codemonkey.ws>  writes:
>
>    
>> On 08/19/2010 04:21 PM, Blue Swirl wrote:
>>      
>>>> Should CPUs appear in the QEMU device tree?
>>>>
>>>>          
>>> They have several properties that should be user visible.
>>>
>>>        
>> Sure, but that's an argument for having some of the qdev features
>> (like variant properties) be consumable outside of qdev.
>>      
> While that might be useful, I don't quite see what makes CPUs so special
> that they need to be kept out of qdev.  Could be just my ignorance, of
> course.
>    

CPUs have special relationships with things like memory in QEMU.  You 
can argue that a device is anything with pins and that CPUs are just 
like any other chip.  But do we really want to model memory chipsets, a 
north and south bridge, and long with cache hierarchies?

It's the trade off between a functional simulator and a cycle accurate 
simulator.

>> The only way to do this right now is having a bus with a single
>> device.  I've been working on the serial device, and what this
>> requires is having an ISASerialDevice which is-a ISADevice.  The
>> ISASerialDevice has-a
>> SerialBus and the SerialBus contain a single SerialDevice which is-a
>> DeviceState.
>>      
> It needs to be a SerialBusDevice, not just a DeviceState, actually.
>    

It doesn't "need to be" actually.  Like it or not, we're still upcast 
from DeviceState so there's no practical reason to have a 
SerialBusDevice unless you're wrapping an interface behind function 
pointers.  But if you're only ever going to have one SerialDevice, 
what's the point?

>> But the ISASerialDevice has to assume that the SerialBus contains only
>> a single child because it needs to connect the GPIO out from the
>> SerialDevice to the ISA irq that's assigned.
>>
>> It would be a whole lot simpler to break the "all devices sit on
>> busses" assumption that we have today and simply have the
>> ISASerialDevice has-a
>> SerialDevice with no additional layers of bus indirection.
>>      
> What's the *practical* benefit of splitting up ISASerialDevice into
> UART-ISA adapter and the UART proper?
>
> The "bus" sitting between the two would be trivial.  If we do that
> everywhere, we'll end up with many different, trivial buses.
>    

If we wanted to add per-device locking based on putting a lock in 
DeviceState that was acquired and released whenever you executed a PIO, 
how would you do that today?

You would convert cpu_register_ioport_* to take a DeviceState in 
serial_init_core.  Sure, you could add a layer of indirection in 
ISASerialDevice, but what about timers?  We would want to implement 
device based timers to do the same thing but again, when we register the 
timers we don't have a DeviceState.

All device callbacks should be based on DeviceState * pointers which 
means if we want to share device code between multiple interfaces (be it 
ISA, PCI, or a SysBus device), we need to have a bus in between.

>> qdev level.  Hot plugging is a very bus specific concept with very bus
>> specific restrictions.  For instance, with PCI, you can only plug at
>> slot granularities whereas today, we don't really model multi-function
>> devices as anything more than a set of unrelated devices.  So there's
>> really no way you could conceptually hot plug a multi-function virtio
>> adapter although you can pretty trivially create one statically.
>>      
> I think it's fair to say that qdev doesn't currently get hot-plugging
> right.
>    

I think we can fix it pretty reasonably though.  I think we can get 
pretty far by adding an interface whereas when creating a device, 
qdev_create calls into the parent Bus to "plug" it in which gives the 
parent bus an opportunity to reject the creation.

We need to indicate to the devices when they have been "realized".  
Devices have (at least) three explicit states, constructing, 
initialized, and realized.  Realized is the point where it can expect 
I/O operations from the guest.  Each bus can then decide when it is 
willing to accept new devices and whether the devices can be hotplugged 
meaningfully.

For instance, sysbus would reject any devices added in the realized 
state whereas the PCI bus would accept devices in either the initialized 
state or the realized state but in the realized state, it would trigger 
a hotplug notification to the guest.  If the PCI address of the device 
didn't make sense (for instance, it was a function on a multi-function 
device), it would be rejected.

>>> What about 486? Or 82489?
>>>
>>>        
>> Don't confuse the local APIC with the PIC or the I/O APIC.
>>
>> The local APIC has always existed in the CPU core.  There is also an
>> I/O APIC which exists outside of the CPU core.  The local APIC was
>> introduced with SMP support.
>>
>> The PIC and IO APIC totally make sense to be modelled as qdev.
>>
>>      
>>>>    It's like modelling a TLB as a
>>>> separate device.
>>>>
>>>>          
>>> That has surely happened in ancient times.
>>>
>>>        
>> Yes, but the programming model was different.
>>
>> Look at the PIC compared to the lapic.  The PIC is programmed via pio
>> at a fixed location.  There is only one PIC and it interacts with the
>> system just like all other devices.  IOW, there is no reference to
>> CPUState.
>>
>> When you look at the local APIC (apic.c) however, you see that it's
>> the only device in the tree that actually interacts with a CPUState.
>> The notion of cpu_get_current_apic() is a good example of the tricks
>> needed to make this work.
>>
>> The local APIC is a cpu feature, not a device.
>>      
> A bit like the UART is a ISASerialDevice feature?
>    

Not at all.  A UART is a chip that exists on a physical board.  The fact 
that the board is a daughter board connected via a signal multiplexing 
bus is an implementation detail that is oblivious to the UART.  It can 
be the same chip regardless of whether it's on an ISA card or directly 
attached to a PIC microcontroller.

A local APIC is transistors and microcode in modern processors.  You 
cannot take a modern local APIC and interface it to a PIC 
microcontroller in any meaningful way.

Regards,

Anthony Liguori

  reply	other threads:[~2010-08-20 18:39 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-12 21:14 [Qemu-devel] [PATCH v2 0/7] APIC/IOAPIC cleanup Blue Swirl
2010-06-13 16:56 ` [Qemu-devel] " Jan Kiszka
2010-06-13 17:03   ` Andreas Färber
2010-06-13 17:53     ` Blue Swirl
2010-06-13 18:17       ` Andreas Färber
2010-06-13 17:49   ` Blue Swirl
2010-08-19 19:33 ` [Qemu-devel] " Anthony Liguori
2010-08-19 20:09   ` Blue Swirl
2010-08-19 20:49     ` Anthony Liguori
2010-08-19 21:21       ` Blue Swirl
2010-08-19 21:51         ` Anthony Liguori
2010-08-19 22:52           ` malc
2010-08-20  1:01             ` Anthony Liguori
2010-08-20 10:00               ` malc
2010-08-20  8:42           ` [Qemu-devel] " Paolo Bonzini
2010-08-20 17:01           ` [Qemu-devel] " Markus Armbruster
2010-08-20 18:38             ` Anthony Liguori [this message]
2010-08-22 20:28               ` Avi Kivity
2010-08-22 21:02                 ` Anthony Liguori
2010-08-23  5:46                   ` Avi Kivity
2010-08-23 13:23                     ` Anthony Liguori
2010-08-23 13:42                       ` Avi Kivity
2010-08-23 13:48                         ` Anthony Liguori
2010-08-23 14:00                           ` Avi Kivity
2010-08-23 14:26                             ` Anthony Liguori
2010-08-23 14:32                               ` Avi Kivity
2010-08-23 14:47                                 ` Anthony Liguori
2010-08-23 15:10                                   ` Markus Armbruster
2010-08-23 16:05                                     ` Anthony Liguori
2010-08-23 17:36                                       ` Markus Armbruster
2010-08-23 17:47                                         ` Anthony Liguori
2010-08-23 18:24                                       ` [Qemu-devel] " Jan Kiszka
2010-08-23 18:29                                         ` Anthony Liguori
2010-08-23 15:14                                   ` [Qemu-devel] " Avi Kivity
2010-08-23 16:02                                     ` Anthony Liguori
2010-08-24  9:51                                       ` Avi Kivity
2010-08-20 19:26           ` Blue Swirl
2010-08-20 10:35       ` [Qemu-devel] " Jan Kiszka
2010-08-22  9:37       ` [Qemu-devel] " Avi Kivity
2010-08-22 18:52         ` Anthony Liguori
2010-08-22 19:44           ` Avi Kivity
2010-08-22 20:03             ` Anthony Liguori
2010-08-22 20:33               ` Avi Kivity
2010-08-22 21:06                 ` Anthony Liguori
2010-08-23  5:49                   ` Avi Kivity
2010-08-23  9:09                     ` [Qemu-devel] " Jan Kiszka
2010-08-23  9:25                       ` Avi Kivity
2010-08-23 10:11                         ` Alexander Graf
2010-08-23 10:15                           ` Avi Kivity
2010-08-23 10:18                             ` Alexander Graf
2010-08-23 10:25                               ` Avi Kivity
2010-08-22 21:07             ` [Qemu-devel] " Anthony Liguori
2010-08-23  5:48               ` Avi Kivity
2010-08-22  9:13   ` Avi Kivity

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4C6ECBB7.7060608@codemonkey.ws \
    --to=anthony@codemonkey.ws \
    --cc=armbru@redhat.com \
    --cc=avi@redhat.com \
    --cc=blauwirbel@gmail.com \
    --cc=jinsong.liu@intel.com \
    --cc=paul@codesourcery.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).