All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Radim Krčmář" <rkrcmar@redhat.com>
To: James Sullivan <sullivan.james.f@gmail.com>
Cc: kvm@vger.kernel.org, pbonzini@redhat.com, gleb@kernel.org
Subject: Re: [Patch v4] x86: irq_comm: Add check for RH bit in kvm_set_msi_irq
Date: Fri, 13 Mar 2015 16:08:29 +0100	[thread overview]
Message-ID: <20150313150829.GB1711@potion.brq.redhat.com> (raw)
In-Reply-To: <5502F885.7080508@gmail.com>

2015-03-13 08:47-0600, James Sullivan:
> On 03/13/2015 08:39 AM, Radim Krčmář wrote:
> ...
> > The warning message is very clever:
> > - it contains the magical "may" qualifier and being protected only by
> >   RH=1 creates weird-looking code structure, but it is technically right
> >   1) lowest-priority delivery may be set in msi.data, which avoids our
> >      otherwise incorrect behavior with RH=1/DM=1
> >   2) RH=1/DM=0 can't deliver to multiple APICs (broadcast is forbidden),
> >      but real hardware may overwrite delivery mode from msi.data
> > - being two lines apart adds to suspicion, yet it can be hint to those
> >   possible problems
> >
> > I only fear it is too clever :)
> >
> 
> For the error message, how does:
> 
> 	kvm: MSI RH=1 unsupported, use low-priority delivery mode
> 
> Sit with you?

I actually liked the former.

New one doesn't say what is the impact of the error and the advice is
not easy follow -- people usually have no idea what low-priority
delivery mode is and nothing can be done outside of the guest.

(I put the rant mainly for future reviewers;  the alternative I had in
 was to warn only when DM=1.)

  reply	other threads:[~2015-03-13 15:08 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-12 23:41 [Patch v3] x86: irq_comm: Add check for RH bit in kvm_set_msi_irq James Sullivan
2015-03-13  2:50 ` James Sullivan
2015-03-13  3:08   ` [Patch v4] " James Sullivan
2015-03-13 14:39     ` Radim Krčmář
2015-03-13 14:47       ` James Sullivan
2015-03-13 15:08         ` Radim Krčmář [this message]
2015-03-13 15:12           ` James Sullivan

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=20150313150829.GB1711@potion.brq.redhat.com \
    --to=rkrcmar@redhat.com \
    --cc=gleb@kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=sullivan.james.f@gmail.com \
    /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.