| Message ID | 20170407085041.5257-4-david@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2017-04-07 10:50+0200, David Hildenbrand: > Let's replace the checks for pic_in_kernel() and ioapic_in_kernel() by > checks against irqchip_mode. > > Also make sure that creation of any route is only possible if we have > an lapic in kernel (irqchip_in_kernel()) or if we are currently > inititalizing the irqchip. > > This is necessary to switch pic_in_kernel() and ioapic_in_kernel() to > irqchip_mode, too. > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > arch/x86/kvm/irq_comm.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c > index 6825cd3..2e5eec8 100644 > --- a/arch/x86/kvm/irq_comm.c > +++ b/arch/x86/kvm/irq_comm.c > @@ -282,24 +282,26 @@ int kvm_set_routing_entry(struct kvm *kvm, > int delta; > unsigned max_pin; > > + /* also allow creation of routes during KVM_IRQCHIP_INIT_IN_PROGRESS */ > + if (kvm->arch.irqchip_mode == KVM_IRQCHIP_NONE) > + goto out; > + > + /* Matches smp_wmb() when setting irqchip_mode */ > + smp_rmb(); This barrier is superfluous as well ... aren't all callers using kvm->lock to provide ordering? The check for KVM_IRQCHIP_NONE would prevent nothing if we could catch the initialization from the outside and hence need a barrier. Thanks.
On 12.04.2017 20:36, Radim Krčmář wrote: > 2017-04-07 10:50+0200, David Hildenbrand: >> Let's replace the checks for pic_in_kernel() and ioapic_in_kernel() by >> checks against irqchip_mode. >> >> Also make sure that creation of any route is only possible if we have >> an lapic in kernel (irqchip_in_kernel()) or if we are currently >> inititalizing the irqchip. >> >> This is necessary to switch pic_in_kernel() and ioapic_in_kernel() to >> irqchip_mode, too. >> >> Signed-off-by: David Hildenbrand <david@redhat.com> >> --- >> arch/x86/kvm/irq_comm.c | 14 ++++++++------ >> 1 file changed, 8 insertions(+), 6 deletions(-) >> >> diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c >> index 6825cd3..2e5eec8 100644 >> --- a/arch/x86/kvm/irq_comm.c >> +++ b/arch/x86/kvm/irq_comm.c >> @@ -282,24 +282,26 @@ int kvm_set_routing_entry(struct kvm *kvm, >> int delta; >> unsigned max_pin; >> >> + /* also allow creation of routes during KVM_IRQCHIP_INIT_IN_PROGRESS */ >> + if (kvm->arch.irqchip_mode == KVM_IRQCHIP_NONE) >> + goto out; >> + >> + /* Matches smp_wmb() when setting irqchip_mode */ >> + smp_rmb(); > > This barrier is superfluous as well ... aren't all callers using > kvm->lock to provide ordering? Yes they are. Paolo suggested this. I think we can safely drop this. Thanks! > > The check for KVM_IRQCHIP_NONE would prevent nothing if we could catch > the initialization from the outside and hence need a barrier. > > Thanks. >
diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c index 6825cd3..2e5eec8 100644 --- a/arch/x86/kvm/irq_comm.c +++ b/arch/x86/kvm/irq_comm.c @@ -282,24 +282,26 @@ int kvm_set_routing_entry(struct kvm *kvm, int delta; unsigned max_pin; + /* also allow creation of routes during KVM_IRQCHIP_INIT_IN_PROGRESS */ + if (kvm->arch.irqchip_mode == KVM_IRQCHIP_NONE) + goto out; + + /* Matches smp_wmb() when setting irqchip_mode */ + smp_rmb(); switch (ue->type) { case KVM_IRQ_ROUTING_IRQCHIP: + if (irqchip_split(kvm)) + goto out; delta = 0; switch (ue->u.irqchip.irqchip) { case KVM_IRQCHIP_PIC_SLAVE: delta = 8; /* fall through */ case KVM_IRQCHIP_PIC_MASTER: - if (!pic_in_kernel(kvm)) - goto out; - e->set = kvm_set_pic_irq; max_pin = PIC_NUM_PINS; break; case KVM_IRQCHIP_IOAPIC: - if (!ioapic_in_kernel(kvm)) - goto out; - max_pin = KVM_IOAPIC_NUM_PINS; e->set = kvm_set_ioapic_irq; break;
Let's replace the checks for pic_in_kernel() and ioapic_in_kernel() by checks against irqchip_mode. Also make sure that creation of any route is only possible if we have an lapic in kernel (irqchip_in_kernel()) or if we are currently inititalizing the irqchip. This is necessary to switch pic_in_kernel() and ioapic_in_kernel() to irqchip_mode, too. Signed-off-by: David Hildenbrand <david@redhat.com> --- arch/x86/kvm/irq_comm.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)