Re: [Qemu-devel] [PATCH 1/2] Keep track of ISA ports ISA device is using in qdev.

[Gleb Natapov <gleb@redhat.com>]

On Mon, Oct 25, 2010 at 08:01:13PM +0200, Markus Armbruster wrote:
> Gleb Natapov <gleb@redhat.com> writes:
> 
> > On Mon, Oct 25, 2010 at 05:06:51PM +0200, Markus Armbruster wrote:
> >> Gleb Natapov <gleb@redhat.com> writes:
> >> 
> >> > Prevent two devices from claiming the same io port.
> >> 
> >> Really?
> >> 
> >> Your new check for double-claim is in the new isa_init_ioport(), which
> >> is for ISADevice only.  Thus, only qdevified ISA devices can opt for
> >> this protection, by calling isa_init_ioport().  It doesn't protect from
> >> devices who don't or can't opt in, such as PCI devices.
> >> 
> > I didn't claim different. This obviously works only for ISA qdev
> > devices.
> 
> I read "Prevent two devices from claiming the same io port" as such.
> 
And have you read subject with that?

> >> Anyway, we already check for double-claim in
> >> register_ioport_{read,write}().  The check has issues --- hw_error() is
> >> wrong there for hot plug.  But it's where the check should be, isn't it?
> > You don't like double-claim checking? Forget about it (all 3 lines
> > of code). The real point of the patch is to have ISA resources used
> > by devices to be stored in common place (ISADevice) which allows
> > get_dev_path() to be implemented.
> 
> We already track I/O port use, in ioport.c.  I don't like that
> duplicated.  Even less so if the dupe catches fewer double-claims than
> the original.
Consider it removed although we do keep track of irqs there and this
tracking is also incomplete. Why is it there?

> 
> Perhaps your new function should wrap around register_ioport_*() instead
> of supplementing it.
register_ioport_*() is disconnected from qdev in general and from ISADEvice
in particular, so how "wrap around register_ioport_*()" will help me to
have get_dev_path() for ISABus is beyond my understanding.

--
			Gleb.