Hi
On Fri, Jan 29, 2016 at 4:59 PM, Markus Armbruster <arm...@redhat.com> wrote:
> marcandre.lur...@redhat.com writes:
>
>> From: Marc-André Lureau <marcandre.lur...@redhat.com>
>>
>> Call ivshmem_setup_interrupts() with or without MSI, always allocate
>> msi_vectors that is going to be used in all case in the following patch.
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
>> ---
>> hw/misc/ivshmem.c | 27 +++++++++++++++++----------
>> 1 file changed, 17 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
>> index dcfc8cc..11780b1 100644
>> --- a/hw/misc/ivshmem.c
>> +++ b/hw/misc/ivshmem.c
>> @@ -768,19 +768,28 @@ static void ivshmem_reset(DeviceState *d)
>> ivshmem_use_msix(s);
>> }
>>
>> -static int ivshmem_setup_msi(IVShmemState * s)
>> +static int ivshmem_setup_interrupts(IVShmemState *s, Error **errp)
>> {
>> - if (msix_init_exclusive_bar(PCI_DEVICE(s), s->vectors, 1)) {
>> - return -1;
>> + /* allocate QEMU callback data for receiving interrupts */
>> + s->msi_vectors = g_malloc0(s->vectors * sizeof(MSIVector));
>> + if (!s->msi_vectors) {
>
> Happens exactly when s->vectors is zero. Is that a legitimate
> configuration?
msix_init_exclusive_bar() already failed with nvectors == 0. However
it is acceptable for !msi to have nvectors == 0. Fixed.
>> + goto fail;
>> }
>>
>> - IVSHMEM_DPRINTF("msix initialized (%d vectors)\n", s->vectors);
>> + if (ivshmem_has_feature(s, IVSHMEM_MSI)) {
>> + if (msix_init_exclusive_bar(PCI_DEVICE(s), s->vectors, 1)) {
>> + goto fail;
>> + }
>>
>> - /* allocate QEMU char devices for receiving interrupts */
>> - s->msi_vectors = g_malloc0(s->vectors * sizeof(MSIVector));
>> + IVSHMEM_DPRINTF("msix initialized (%d vectors)\n", s->vectors);
>> + ivshmem_use_msix(s);
>> + }
>>
>> - ivshmem_use_msix(s);
>> return 0;
>> +
>> +fail:
>> + error_setg(errp, "failed to initialize interrupts");
>> + return -1;
>> }
>
> Recommend not to move the error_setg(). Keeps this function simpler, at
> no cost.
ok
>
>>
>> static void ivshmem_enable_irqfd(IVShmemState *s)
>> @@ -946,9 +955,7 @@ static void pci_ivshmem_realize(PCIDevice *dev, Error
>> **errp)
>> IVSHMEM_DPRINTF("using shared memory server (socket = %s)\n",
>> s->server_chr->filename);
>>
>> - if (ivshmem_has_feature(s, IVSHMEM_MSI) &&
>> - ivshmem_setup_msi(s)) {
>> - error_setg(errp, "msix initialization failed");
>> + if (ivshmem_setup_interrupts(s, errp) < 0) {
>> return;
>> }
>
> Yup, the only change is we now allocate s->msi_vectors whether we have
> IVSHMEM_MSI or not.
>
--
Marc-André Lureau
