Re: [Qemu-devel] [PATCH 0/2] Add get_dev_path callback to ISA bus.

[Gleb Natapov <gleb@redhat.com>]

On Tue, Oct 26, 2010 at 01:04:18PM +0200, Markus Armbruster wrote:
> Gleb Natapov <gleb@redhat.com> writes:
> 
> > On Tue, Oct 26, 2010 at 11:57:53AM +0200, Markus Armbruster wrote:
> >> Gleb Natapov <gleb@redhat.com> writes:
> >> 
> >> > On Mon, Oct 25, 2010 at 07:53:41PM +0200, Markus Armbruster wrote:
> >> >> Gleb Natapov <gleb@redhat.com> writes:
> >> >> 
> >> >> > On Mon, Oct 25, 2010 at 05:10:15PM +0200, Markus Armbruster wrote:
> >> >> >> Gleb Natapov <gleb@redhat.com> writes:
> >> >> >> 
> >> >> >> > PCI bus already has one. For ISA bus this patch series uses device's
> >> >> >> > ioports to uniquely describe it. For isa-ide, for example, get_dev_path
> >> >> >> > method returns:
> >> >> >> > 01f0-01f7,03f6 for first IDE controller
> >> >> >> > 0170-0177,0376 for second one
> >> >> >> 
> >> >> >> Any I/O port used by the device identifies it.  I'd say a common
> >> >> >> identifier is the "I/O base", the lowest I/O port used.
> >> >> > So use only first port from the string. More information is better then
> >> >> > less information. You can always drop information you do not need.
> >> >> 
> >> >> I'd prefer canonical bus addresses to be terse.  It's not the place to
> >> >> give additional information.
> >> >
> >> > I'd prefer them to give full info but not more. I don't see why you
> >> > point is more valid then mine.
> >> 
> >> Full information about ISA resources is more than I/O ports, it also
> >> includes IRQs and DMA channels.
> > So you are arguing to add more info there? :) Theoretically there is
> > nothing that prevents ISA devices from sharing IRQ line (existing ISA
> > devices just not built to do that and nobody produces new once) and
> > DMA is separate device on ISA bus AFAIK.
> >
> >> 
> >> An address is not the place to give full information.  The purpose of an
> >> address is to name a thing, not to give full information about that
> >> thing.
> >> 
> > Address should contain info that allows to find device easily. If
> > whoever looking for a device knows only one port device is using he
> > will not find the device if we'll use different port to name it. May be
> > not the problem in practice.
> 
> If he knows only one port, he can't form the address proposed by you.
He doesn't need to. He scans all devices and search for one that
list this ioport in its bus name. 

> He needs to know *all* ports.  
Why? If there is device ISA@0000-0005,0008 and I want to know what
devices uses port 0003 I can easily find it.

>                                So what he does is search all the devices
> for the one that uses the port in question.  Same for the address I
> propose.  The proposal differ for the case where the user remembers just
> the base address.  I believe that's the common case.
> 
> >> By the way, get_dev_path() really needs a written contract.  Actually,
> >> all the qdev and qbus callbacks do.
> > What do you mean by that? It should be stable ABI, is this what you
> > mean?
> 
> A contract between qdev core and the device models, so that device model
> authors know what exactly their callback functions are supposed to do.
I thought that was worked out when get_dev_path() callback was
introduced.

> 
> Stability is a separate matter.  Since it's a purely internal interface,
> we don't want it.
I think that where your misunderstanding of most of my patches are
coming from :( The way I am going to use it it will not be internal
interface anymore. It will be ABI between qemu and a guest.

--
			Gleb.