[v3,03/24] KVM: x86: check against irqchip_mode in kvm_set_routing_entry()
diff mbox

Message ID 20170407085041.5257-4-david@redhat.com
State New
Headers show

Commit Message

David Hildenbrand April 7, 2017, 8:50 a.m. UTC
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(-)

Comments

Radim Krčmář April 12, 2017, 6:36 p.m. UTC | #1
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.
David Hildenbrand April 12, 2017, 7:56 p.m. UTC | #2
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.
>

Patch
diff mbox

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;