[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] Keep track of ISA ports ISA device is using
From: |
Gleb Natapov |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] Keep track of ISA ports ISA device is using in qdev. |
Date: |
Tue, 26 Oct 2010 12:29:14 +0200 |
On Tue, Oct 26, 2010 at 11:49:26AM +0200, Markus Armbruster wrote:
> Gleb Natapov <address@hidden> writes:
>
> > On Mon, Oct 25, 2010 at 08:01:13PM +0200, Markus Armbruster wrote:
> >> Gleb Natapov <address@hidden> writes:
> >>
> >> > On Mon, Oct 25, 2010 at 05:06:51PM +0200, Markus Armbruster wrote:
> >> >> Gleb Natapov <address@hidden> 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?
>
> Yes. Nevertheless, I found it a bit misleading, so I commented.
>
Well, I hope it is clear now.
> >> >> 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?
>
> Where's "there"?
>
In ISADevice. Look for "isabus->assigned" in isa-bus.c.
> Let's not add to the code duplication if we can help it. It's bad
> enough as it is.
>
> >> 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.
>
> I was too terse, sorry.
>
> Maybe we should have functions for ISADevices to register ISA resources.
> Functions that are *not* disconnected from qdev. Instead of code like
>
> register_ioport_write(pm_io_base, 64, 2, pm_ioport_writew, s);
> register_ioport_read(pm_io_base, 64, 2, pm_ioport_readw, s);
> register_ioport_write(pm_io_base, 64, 4, pm_ioport_writel, s);
> register_ioport_read(pm_io_base, 64, 4, pm_ioport_readl, s);
> isa_init_ioport_range(dev, pm_ioport_base, 64);
>
> or, with Avi's proposed interface
>
> ioport_register(&s->ioport, pm_io_base, 64);
> isa_init_ioport_range(dev, pm_ioport_base, 64);
>
> we'd get something like
>
> isa_ioport_register(dev, &s->ioport, pm_io_base, 64);
>
> Isn't that neater?
>
Neater, but out of scope for my work. Obviously all resource
management should go via qdev at the end.
> Since some PCI devices also register ISA resources, we'd have to
> offer functions for them as well, properly integrated.
>
> Now, I didn't mean to ask you to do all that as a precondition for
> getting your patch in. I was just observing.
OK.
--
Gleb.
Re: [Qemu-devel] [PATCH 0/2] Add get_dev_path callback to ISA bus., Markus Armbruster, 2010/10/25
- Re: [Qemu-devel] [PATCH 0/2] Add get_dev_path callback to ISA bus., Gleb Natapov, 2010/10/25
- Re: [Qemu-devel] [PATCH 0/2] Add get_dev_path callback to ISA bus., Markus Armbruster, 2010/10/25
- Re: [Qemu-devel] [PATCH 0/2] Add get_dev_path callback to ISA bus., Gleb Natapov, 2010/10/25
- Re: [Qemu-devel] [PATCH 0/2] Add get_dev_path callback to ISA bus., Markus Armbruster, 2010/10/26
- Re: [Qemu-devel] [PATCH 0/2] Add get_dev_path callback to ISA bus., Gleb Natapov, 2010/10/26
- Re: [Qemu-devel] [PATCH 0/2] Add get_dev_path callback to ISA bus., Markus Armbruster, 2010/10/26
- Re: [Qemu-devel] [PATCH 0/2] Add get_dev_path callback to ISA bus., Gleb Natapov, 2010/10/26