[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/3] Add PCI memory region registration
From: |
Anthony Liguori |
Subject: |
Re: [Qemu-devel] [PATCH 2/3] Add PCI memory region registration |
Date: |
Sun, 31 May 2009 08:52:14 -0500 |
User-agent: |
Thunderbird 2.0.0.21 (X11/20090409) |
Avi Kivity wrote:
>
> I wanted to make it an io_region as well, but pci_register_io_region()
> is taken. We could rename it to pci_register_bar(), but that will
> cause too much churn IMO.
bar is okay by me.
>> It overloads the term physical memory and still requires separate
>> APIs for IO regions and MEM regions. I know you mentioned that
>> ram_addr_t could be overloaded to also support IO regions but IMHO
>> that's rather confusing. If the new code looked like:
>>
>> s->rtl8139_mmio_io_addr =
>> cpu_register_io_memory(0, rtl8139_mmio_read, rtl8139_mmio_write, s);
>>
>> s->rtl8139_io_io_addr =
>> cpu_register_io_memory(0, rtl8139_ioport_read,
>> rtl8139_ioport_write, s);
>>
>> pci_register_io_region(&d->dev, 0, 0x100,
>> PCI_ADDRESS_SPACE_IO, s->rtl8139_io_io_addr);
>>
>> pci_register_io_region(&d->dev, 1, 0x100,
>> PCI_ADDRESS_SPACE_MEM, s->rtl8139_mmio_addr);
>>
>> I think it would be more understandable. However, I think that the
>> normal case is exactly this work flow so I think it makes sense to
>> collapse it into two function calls. So it could look like:
>>
>> pci_register_io_region(&d->dev, 0, 0x100,
>> PCI_ADDRESS_SPACE_IO, rtl8139_ioport_read,
>> rtl8139_ioport_write, s);
>>
>> pci_register_io_region(&d->dev, 1, 0x100,
>> PCI_ADDRESS_SPACE_MEM, rtl8139_mmio_read,
>> rtl8139_mmio_write, s);
>
> That neglects the case of direct mapping. We could add it as a helper
> though (which would also need to unregister the ram_addr when the
> io_region is unregistered).
I was thinking that the first API (which is very similar to your
original API) would be the implementation with the second API being
helpers that everyone actually used.
>> You could still have a two step process for where it's absolutely
>> required (like VGA optimization).
>>
>> I think it's worth looking at changing the signatures of the mem
>> read/write functions. Introducing a size parameter would greatly
>> simplify adding 64-bit IO support, for instance.
>
> Well, let's separate those unrelated changes, otherwise nothing will
> get done.
As long as we agree on the final API, we can take incremental steps there.
>> I would argue that ram_addr_t is the wrong thing to overload for PIO
>> but as long as it's not exposed in the common API, it doesn't matter
>> that much to me.
>
> Currently ram_addr_t is really a 64-bit encoding for a QEMUIORegion
> object, plus support for address arithmetic. IMO we should drop it
> and move towards explicit object representation, and drop the page
> descriptor array.
>
> One of the things that prevents this is that the page descriptor array
> is the only place holding the physical -> ioregion mapping. Once we
> move this information to other objects, we can start on that. Hence
> this patchset.
So let's pick a better name then physical_memory and I think we've got a
good starting point.
Regards,
Anthony Liguori
- Re: [Qemu-devel] [PATCH 1/3] Add PhysicalMemoryRegion type, (continued)
- [Qemu-devel] [PATCH 2/3] Add PCI memory region registration, Avi Kivity, 2009/05/24
- Re: [Qemu-devel] [PATCH 2/3] Add PCI memory region registration, Anthony Liguori, 2009/05/27
- Re: [Qemu-devel] [PATCH 2/3] Add PCI memory region registration, Avi Kivity, 2009/05/27
- Re: [Qemu-devel] [PATCH 2/3] Add PCI memory region registration, Anthony Liguori, 2009/05/27
- Re: [Qemu-devel] [PATCH 2/3] Add PCI memory region registration, Avi Kivity, 2009/05/27
- Re: [Qemu-devel] [PATCH 2/3] Add PCI memory region registration, Anthony Liguori, 2009/05/27
- Re: [Qemu-devel] [PATCH 2/3] Add PCI memory region registration, Christoph Hellwig, 2009/05/31
- Re: [Qemu-devel] [PATCH 2/3] Add PCI memory region registration, Avi Kivity, 2009/05/31
- Re: [Qemu-devel] [PATCH 2/3] Add PCI memory region registration, Avi Kivity, 2009/05/31
- Re: [Qemu-devel] [PATCH 2/3] Add PCI memory region registration,
Anthony Liguori <=
- Re: [Qemu-devel] [PATCH 2/3] Add PCI memory region registration, Avi Kivity, 2009/05/31
[Qemu-devel] [PATCH 3/3] Convert RTL8139 to use PCI memory regitration facility, Avi Kivity, 2009/05/24
Re: [Qemu-devel] [PATCH 0/3] Object-based physical memory management, Anthony Liguori, 2009/05/27