From: Gleb Natapov <gleb@redhat.com>
To: Marcelo Tosatti <mtosatti@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>,
Avi Kivity <avi@redhat.com>,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH repost] kvm: drop parameter validation
Date: Tue, 14 Aug 2012 07:06:08 +0300 [thread overview]
Message-ID: <20120814040608.GA16588@redhat.com> (raw)
In-Reply-To: <20120813205932.GA15395@amt.cnet>
On Mon, Aug 13, 2012 at 05:59:32PM -0300, Marcelo Tosatti wrote:
> On Mon, Aug 13, 2012 at 05:30:53PM +0300, Gleb Natapov wrote:
> > On Mon, Aug 13, 2012 at 01:43:58PM +0300, Michael S. Tsirkin wrote:
> > > We validate irq pin number when routing is setup, so
> > > code handling illegal irq # in pic and ioapic on each injection
> > > is never called.
> > > Drop it.
> > >
> > I would leave BUG_ON there for a while.
>
> Which BUG_ON?
>
I mean changing if() to BUG_ON().
BUG_ON (!(irq >= 0 && irq < PIC_NUM_PINS))
> >
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > ---
> > >
> > > Reposting, applies without changes to kvm/next.
> > >
> > > arch/x86/kvm/i8259.c | 16 +++++++---------
> > > virt/kvm/ioapic.c | 35 +++++++++++++++++------------------
> > > 2 files changed, 24 insertions(+), 27 deletions(-)
> > >
> > > diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
> > > index 1df8fb9..277ec0d 100644
> > > --- a/arch/x86/kvm/i8259.c
> > > +++ b/arch/x86/kvm/i8259.c
> > > @@ -190,17 +190,15 @@ void kvm_pic_update_irq(struct kvm_pic *s)
> > >
> > > int kvm_pic_set_irq(struct kvm_pic *s, int irq, int irq_source_id, int level)
> > > {
> > > - int ret = -1;
> > > + int ret, irq_level;
> > >
> > > pic_lock(s);
> > > - if (irq >= 0 && irq < PIC_NUM_PINS) {
> > > - int irq_level = __kvm_irq_line_state(&s->irq_states[irq],
> > > - irq_source_id, level);
> > > - ret = pic_set_irq1(&s->pics[irq >> 3], irq & 7, irq_level);
> > > - pic_update_irq(s);
> > > - trace_kvm_pic_set_irq(irq >> 3, irq & 7, s->pics[irq >> 3].elcr,
> > > - s->pics[irq >> 3].imr, ret == 0);
> > > - }
> > > + irq_level = __kvm_irq_line_state(&s->irq_states[irq],
> > > + irq_source_id, level);
> > > + ret = pic_set_irq1(&s->pics[irq >> 3], irq & 7, irq_level);
> > > + pic_update_irq(s);
> > > + trace_kvm_pic_set_irq(irq >> 3, irq & 7, s->pics[irq >> 3].elcr,
> > > + s->pics[irq >> 3].imr, ret == 0);
> > > pic_unlock(s);
> > >
> > > return ret;
> > > diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
> > > index ef61d52..4d824c7 100644
> > > --- a/virt/kvm/ioapic.c
> > > +++ b/virt/kvm/ioapic.c
> > > @@ -197,28 +197,27 @@ int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int irq_source_id,
> > > u32 old_irr;
> > > u32 mask = 1 << irq;
> > > union kvm_ioapic_redirect_entry entry;
> > > - int ret = 1;
> > > + int ret, irq_level;
> > >
> > > spin_lock(&ioapic->lock);
> > > old_irr = ioapic->irr;
> > > - if (irq >= 0 && irq < IOAPIC_NUM_PINS) {
> > > - int irq_level = __kvm_irq_line_state(&ioapic->irq_states[irq],
> > > - irq_source_id, level);
> > > - entry = ioapic->redirtbl[irq];
> > > - irq_level ^= entry.fields.polarity;
> > > - if (!irq_level)
> > > - ioapic->irr &= ~mask;
> > > - else {
> > > - int edge = (entry.fields.trig_mode == IOAPIC_EDGE_TRIG);
> > > - ioapic->irr |= mask;
> > > - if ((edge && old_irr != ioapic->irr) ||
> > > - (!edge && !entry.fields.remote_irr))
> > > - ret = ioapic_service(ioapic, irq);
> > > - else
> > > - ret = 0; /* report coalesced interrupt */
> > > - }
> > > - trace_kvm_ioapic_set_irq(entry.bits, irq, ret == 0);
> > > + irq_level = __kvm_irq_line_state(&ioapic->irq_states[irq],
> > > + irq_source_id, level);
> > > + entry = ioapic->redirtbl[irq];
> > > + irq_level ^= entry.fields.polarity;
> > > + if (!irq_level) {
> > > + ioapic->irr &= ~mask;
> > > + ret = 1;
> > > + } else {
> > > + int edge = (entry.fields.trig_mode == IOAPIC_EDGE_TRIG);
> > > + ioapic->irr |= mask;
> > > + if ((edge && old_irr != ioapic->irr) ||
> > > + (!edge && !entry.fields.remote_irr))
> > > + ret = ioapic_service(ioapic, irq);
> > > + else
> > > + ret = 0; /* report coalesced interrupt */
> > > }
> > > + trace_kvm_ioapic_set_irq(entry.bits, irq, ret == 0);
> > > spin_unlock(&ioapic->lock);
> > >
> > > return ret;
> > > --
> > > MST
> >
> > --
> > Gleb.
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Gleb.
next prev parent reply other threads:[~2012-08-14 4:06 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-08-13 10:43 [PATCH repost] kvm: drop parameter validation Michael S. Tsirkin
2012-08-13 14:30 ` Gleb Natapov
2012-08-13 20:59 ` Marcelo Tosatti
2012-08-14 4:06 ` Gleb Natapov [this message]
2012-08-14 8:34 ` 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=20120814040608.GA16588@redhat.com \
--to=gleb@redhat.com \
--cc=avi@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mst@redhat.com \
--cc=mtosatti@redhat.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.