All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: Cao jin <caoj.fnst@cn.fujitsu.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>,
	mst@redhat.com, jasowang@redhat.com, qemu-devel@nongnu.org,
	alex.williamson@redhat.com, hare@suse.de, dmitry@daynix.com,
	pbonzini@redhat.com, jsnow@redhat.com, kraxel@redhat.com
Subject: Re: [Qemu-devel] [PATCH v4 5/5] Add param Error ** for msi_init()
Date: Fri, 29 Apr 2016 14:46:59 +0200	[thread overview]
Message-ID: <8737q41t18.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <5723294F.2000501@cn.fujitsu.com> (Cao jin's message of "Fri, 29 Apr 2016 17:28:47 +0800")

Cao jin <caoj.fnst@cn.fujitsu.com> writes:

> Hi Markus,
>     sorry for replying so late, I am stucked by other tasks for a while.
>
> On 04/12/2016 07:50 PM, Markus Armbruster wrote:
>>
>> Examine how it uses msi_init().  That's how we give a PCI device
>> capability MSI.  If the device model treats msi_init() failure as fatal,
>> it doesn't have a non-MSI variant.
>>
>> Some device models let the user ask for MSI with a property, and make
>> the msi_init() call depend on the property.  The property is commonly
>> called "msi" or "use_msi", with values "on" and "off".
>>
>> msi=off then works as expected: you get the variant without MSI.
>>
>> The meaning of msi=on depends on how msi_init() failure is handled.  If
>> it's fatal, then msi=on works as expected: you get the variant with MSI.
>> But if it's not fatal, then you may or may not get it, which I consider
>> a grossly misleading user interface.
>>
>> To clean this up, we could add msi=auto, and move the non-fatal behavior
>> from msi=on to msi=auto.
>>
>> Same for MSI-X with msix_init() and property "msix".
>>
> Let me try to understand your meaning, correct me pls if I am wrong:
>
> replace the msi property type from bit/int to enum OnOffAuto(it seems
> need a tiny surgery for device structure), and default to auto. Then
> process going to look like this:
>
> (msi property = auto) means enable msi by default. If msi_init fail,
> we switch to the non-MSI variant; If msi_init success, we got msi
> variant.

You got it.

> Another condition I want to mention, ahci, pvscsi & vmxnet3 don`t have
> msi property, and when msi_init fail, they all will use intx. One
> thing need to be confirmed: whether they need a msi property or not?

While I don't particularly like additional configuration knobs, I like
the "try to use MSI, fall back to INTx silently" behavior even less.
I'd welcome such a property.  Can guarantee the respective maintainers
will agree with me, of course.

  reply	other threads:[~2016-04-29 12:47 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-05 11:26 [Qemu-devel] [PATCH v4 0/5] Add param Error ** for msi_init() Cao jin
2016-04-05 11:26 ` [Qemu-devel] [PATCH v4 1/5] fix some coding style problems Cao jin
2016-04-08  6:29   ` Markus Armbruster
2016-04-09  8:49     ` Cao jin
2016-04-05 11:26 ` [Qemu-devel] [PATCH v4 2/5] change pvscsi_init_msi() type to void Cao jin
2016-04-06  7:19   ` Dmitry Fleytman
2016-04-10  7:41   ` Marcel Apfelbaum
2016-04-05 11:26 ` [Qemu-devel] [PATCH v4 3/5] megasas: bugfix Cao jin
2016-04-08  7:16   ` Markus Armbruster
2016-04-09 13:07     ` Cao jin
2016-04-10  7:40     ` Marcel Apfelbaum
2016-04-05 11:26 ` [Qemu-devel] [PATCH v4 4/5] mptsas: change .realize function name Cao jin
2016-04-10  7:43   ` Marcel Apfelbaum
2016-04-05 11:26 ` [Qemu-devel] [PATCH v4 5/5] Add param Error ** for msi_init() Cao jin
2016-04-08  8:44   ` Markus Armbruster
2016-04-09 12:19     ` Cao jin
2016-04-09 13:00       ` Cao jin
2016-04-10  8:20       ` Marcel Apfelbaum
2016-04-10  9:38         ` Cao jin
2016-04-11 10:00           ` Marcel Apfelbaum
2016-04-11 12:02             ` Cao jin
2016-04-12 11:50             ` Markus Armbruster
2016-04-29  9:28               ` Cao jin
2016-04-29 12:46                 ` Markus Armbruster [this message]
2016-04-12  8:34       ` Markus Armbruster
2016-04-05 11:27 ` [Qemu-devel] [PATCH v4 0/5] " Michael S. Tsirkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8737q41t18.fsf@dusky.pond.sub.org \
    --to=armbru@redhat.com \
    --cc=alex.williamson@redhat.com \
    --cc=caoj.fnst@cn.fujitsu.com \
    --cc=dmitry@daynix.com \
    --cc=hare@suse.de \
    --cc=jasowang@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=marcel@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.