On 2012-06-13 13:21, Michael S. Tsirkin wrote:
> On Wed, Jun 13, 2012 at 12:44:01PM +0200, Jan Kiszka wrote:
>> On 2012-06-12 22:03, Alex Williamson wrote:
>>> msix_init has very little configurability as to how it lays out MSIX
>>> for a device. It claims to resize BARs, but doesn't actually do this
>>> anymore. This patch allows MSIX to be fully specified, which is
>>> necessary both for emulated devices trying to match the physical
>>> layout of a hardware device as well as for any kind of device
>>> assignment.
>>>
>>> New functions msix_init_bar & msix_uninit_bar provide wrappers around
>>> the more detailed functions for drivers that just want a simple MSIX
>>> setup.
>>>
>>> Signed-off-by: Alex Williamson <alex.william...@redhat.com>
>>> ---
>>>
>>> hw/ivshmem.c | 9 +-
>>> hw/msix.c | 299
>>> +++++++++++++++++++++++++++++++------------------------
>>> hw/msix.h | 11 +-
>>> hw/pci.h | 12 ++
>>> hw/virtio-pci.c | 15 +--
>>> 5 files changed, 192 insertions(+), 154 deletions(-)
>>>
>>> diff --git a/hw/ivshmem.c b/hw/ivshmem.c
>>> index 05559b6..71c84a6 100644
>>> --- a/hw/ivshmem.c
>>> +++ b/hw/ivshmem.c
>>> @@ -563,16 +563,13 @@ static uint64_t ivshmem_get_size(IVShmemState * s) {
>>>
>>> static void ivshmem_setup_msi(IVShmemState * s)
>>> {
>>> - memory_region_init(&s->msix_bar, "ivshmem-msix", 4096);
>>> - if (!msix_init(&s->dev, s->vectors, &s->msix_bar, 1, 0)) {
>>> - pci_register_bar(&s->dev, 1, PCI_BASE_ADDRESS_SPACE_MEMORY,
>>> - &s->msix_bar);
>>> - IVSHMEM_DPRINTF("msix initialized (%d vectors)\n", s->vectors);
>>> - } else {
>>> + if (msix_init_bar(&s->dev, s->vectors, &s->msix_bar, 1,
>>> "ivshmem-msix")) {
>>
>> I don't think the callers of msix_init_bar should have to provide the
>> memory region for that bar. That can be embedded into PCIDevice, just
>> like you did for the table and PBA. That was my idea with msix_init_simple.
>>
>> Back then, I only included a generic memory region name. That can be
>> improved, but without bothering the caller. Just derive it from
>> PCIDevice::name.
>>
>> Jan
>
> I think callers must initialize the BAR regions.
> This is because BAR can include other stuff besides MSI-X.
> MSI-X adds its own subregion.
That's the non-common case handled by msix_init. I don't see this as
typical for emulated devices.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
