From: marc.zyngier@arm.com (Marc Zyngier)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v5 07/12] ARM: KVM: VGIC virtual CPU interface management
Date: Tue, 15 Jan 2013 11:00:44 +0000 [thread overview]
Message-ID: <50F536DC.2060702@arm.com> (raw)
In-Reply-To: <CANM98qJ_Pb5bT0k7bAhv6i1YTj6GxvNE6vvY49OZrqsKpF3xOA@mail.gmail.com>
On 14/01/13 22:02, Christoffer Dall wrote:
> On Mon, Jan 14, 2013 at 10:42 AM, Will Deacon <will.deacon@arm.com> wrote:
>> On Tue, Jan 08, 2013 at 06:42:11PM +0000, Christoffer Dall wrote:
>>> From: Marc Zyngier <marc.zyngier@arm.com>
>>>
>>> Add VGIC virtual CPU interface code, picking pending interrupts
>>> from the distributor and stashing them in the VGIC control interface
>>> list registers.
>>>
>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>>> Signed-off-by: Christoffer Dall <c.dall@virtualopensystems.com>
>>> ---
>>> arch/arm/include/asm/kvm_vgic.h | 30 ++++
>>> arch/arm/kvm/vgic.c | 327 +++++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 356 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/include/asm/kvm_vgic.h b/arch/arm/include/asm/kvm_vgic.h
>>> index 9ff0d9c..b3133c4 100644
>>> --- a/arch/arm/include/asm/kvm_vgic.h
>>> +++ b/arch/arm/include/asm/kvm_vgic.h
>>> @@ -110,8 +110,33 @@ struct vgic_dist {
>>> };
>>>
>>> struct vgic_cpu {
>>> +#ifdef CONFIG_KVM_ARM_VGIC
>>> + /* per IRQ to LR mapping */
>>> + u8 vgic_irq_lr_map[VGIC_NR_IRQS];
>>> +
>>> + /* Pending interrupts on this VCPU */
>>> + DECLARE_BITMAP( pending_percpu, VGIC_NR_PRIVATE_IRQS);
>>> + DECLARE_BITMAP( pending_shared, VGIC_NR_SHARED_IRQS);
>>> +
>>> + /* Bitmap of used/free list registers */
>>> + DECLARE_BITMAP( lr_used, 64);
>>> +
>>> + /* Number of list registers on this CPU */
>>> + int nr_lr;
>>> +
>>> + /* CPU vif control registers for world switch */
>>> + u32 vgic_hcr;
>>> + u32 vgic_vmcr;
>>> + u32 vgic_misr; /* Saved only */
>>> + u32 vgic_eisr[2]; /* Saved only */
>>> + u32 vgic_elrsr[2]; /* Saved only */
>>> + u32 vgic_apr;
>>> + u32 vgic_lr[64]; /* A15 has only 4... */
>>
>> Have a #define for the maximum number of list registers.
>>
>>> +#endif
>>> };
>>>
>>> +#define LR_EMPTY 0xff
>>> +
>>> struct kvm;
>>> struct kvm_vcpu;
>>> struct kvm_run;
>>> @@ -119,9 +144,14 @@ struct kvm_exit_mmio;
>>>
>>> #ifdef CONFIG_KVM_ARM_VGIC
>>> int kvm_vgic_set_addr(struct kvm *kvm, unsigned long type, u64 addr);
>>> +void kvm_vgic_sync_to_cpu(struct kvm_vcpu *vcpu);
>>> +void kvm_vgic_sync_from_cpu(struct kvm_vcpu *vcpu);
>>
>> Same comment as for the arch timer (flush/sync).
>>
>>> +int kvm_vgic_vcpu_pending_irq(struct kvm_vcpu *vcpu);
>>> bool vgic_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
>>> struct kvm_exit_mmio *mmio);
>>>
>>> +#define irqchip_in_kernel(k) (!!((k)->arch.vgic.vctrl_base))
>>> +
>>> #else
>>> static inline int kvm_vgic_hyp_init(void)
>>> {
>>> diff --git a/arch/arm/kvm/vgic.c b/arch/arm/kvm/vgic.c
>>> index bd2bd7f..58237d5 100644
>>> --- a/arch/arm/kvm/vgic.c
>>> +++ b/arch/arm/kvm/vgic.c
>>> @@ -152,6 +152,34 @@ static int vgic_irq_is_enabled(struct kvm_vcpu *vcpu, int irq)
>>> return vgic_bitmap_get_irq_val(&dist->irq_enabled, vcpu->vcpu_id, irq);
>>> }
>>>
>>> +static int vgic_irq_is_active(struct kvm_vcpu *vcpu, int irq)
>>> +{
>>> + struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
>>> +
>>> + return vgic_bitmap_get_irq_val(&dist->irq_active, vcpu->vcpu_id, irq);
>>> +}
>>> +
>>> +static void vgic_irq_set_active(struct kvm_vcpu *vcpu, int irq)
>>> +{
>>> + struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
>>> +
>>> + vgic_bitmap_set_irq_val(&dist->irq_active, vcpu->vcpu_id, irq, 1);
>>> +}
>>> +
>>> +static void vgic_irq_clear_active(struct kvm_vcpu *vcpu, int irq)
>>> +{
>>> + struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
>>> +
>>> + vgic_bitmap_set_irq_val(&dist->irq_active, vcpu->vcpu_id, irq, 0);
>>> +}
>>> +
>>> +static int vgic_dist_irq_is_pending(struct kvm_vcpu *vcpu, int irq)
>>> +{
>>> + struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
>>> +
>>> + return vgic_bitmap_get_irq_val(&dist->irq_state, vcpu->vcpu_id, irq);
>>> +}
>>> +
>>> static void vgic_dist_irq_set(struct kvm_vcpu *vcpu, int irq)
>>> {
>>> struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
>>> @@ -711,7 +739,30 @@ static void vgic_dispatch_sgi(struct kvm_vcpu *vcpu, u32 reg)
>>>
>>> static int compute_pending_for_cpu(struct kvm_vcpu *vcpu)
>>> {
>>> - return 0;
>>> + struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
>>> + unsigned long *pending, *enabled, *pend_percpu, *pend_shared;
>>> + unsigned long pending_private, pending_shared;
>>> + int vcpu_id;
>>> +
>>> + vcpu_id = vcpu->vcpu_id;
>>> + pend_percpu = vcpu->arch.vgic_cpu.pending_percpu;
>>> + pend_shared = vcpu->arch.vgic_cpu.pending_shared;
>>> +
>>> + pending = vgic_bitmap_get_cpu_map(&dist->irq_state, vcpu_id);
>>> + enabled = vgic_bitmap_get_cpu_map(&dist->irq_enabled, vcpu_id);
>>> + bitmap_and(pend_percpu, pending, enabled, VGIC_NR_PRIVATE_IRQS);
>>> +
>>> + pending = vgic_bitmap_get_shared_map(&dist->irq_state);
>>> + enabled = vgic_bitmap_get_shared_map(&dist->irq_enabled);
>>> + bitmap_and(pend_shared, pending, enabled, VGIC_NR_SHARED_IRQS);
>>> + bitmap_and(pend_shared, pend_shared,
>>> + vgic_bitmap_get_shared_map(&dist->irq_spi_target[vcpu_id]),
>>> + VGIC_NR_SHARED_IRQS);
>>> +
>>> + pending_private = find_first_bit(pend_percpu, VGIC_NR_PRIVATE_IRQS);
>>> + pending_shared = find_first_bit(pend_shared, VGIC_NR_SHARED_IRQS);
>>> + return (pending_private < VGIC_NR_PRIVATE_IRQS ||
>>> + pending_shared < VGIC_NR_SHARED_IRQS);
>>> }
>>>
>>> /*
>>> @@ -737,6 +788,280 @@ static void vgic_update_state(struct kvm *kvm)
>>> }
>>> }
>>>
>>> +#define LR_CPUID(lr) \
>>> + (((lr) & GICH_LR_PHYSID_CPUID) >> GICH_LR_PHYSID_CPUID_SHIFT)
>>> +#define MK_LR_PEND(src, irq) \
>>> + (GICH_LR_PENDING_BIT | ((src) << GICH_LR_PHYSID_CPUID_SHIFT) | (irq))
>>> +/*
>>> + * Queue an interrupt to a CPU virtual interface. Return true on success,
>>> + * or false if it wasn't possible to queue it.
>>> + */
>>> +static bool vgic_queue_irq(struct kvm_vcpu *vcpu, u8 sgi_source_id, int irq)
>>> +{
>>> + struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
>>> + int lr;
>>> +
>>> + /* Sanitize the input... */
>>> + BUG_ON(sgi_source_id & ~7);
>>> + BUG_ON(sgi_source_id && irq > 15);
>>
>> You can use your new NR_SGIS definition here.
>>
>
> This should address the remaining comments:
>
> commit 43957095ec5476beb198f4c4630dfc3e2f3951db
> Author: Christoffer Dall <c.dall@virtualopensystems.com>
> Date: Mon Jan 14 16:59:38 2013 -0500
>
> KVM: ARM: vgic: Define VGIC_MAX_LRS
>
> Define maximum number of link registers we can handle instead of using
> literals in the code. If an architecture reports more link registers
> than we support, only use the number we can support.
>
> Signed-off-by: Christoffer Dall <c.dall@virtualopensystems.com>
>
> diff --git a/arch/arm/include/asm/kvm_vgic.h b/arch/arm/include/asm/kvm_vgic.h
> index 1ace491..f9d1977 100644
> --- a/arch/arm/include/asm/kvm_vgic.h
> +++ b/arch/arm/include/asm/kvm_vgic.h
> @@ -33,6 +33,7 @@
> #define VGIC_NR_PRIVATE_IRQS (VGIC_NR_SGIS + VGIC_NR_PPIS)
> #define VGIC_NR_SHARED_IRQS (VGIC_NR_IRQS - VGIC_NR_PRIVATE_IRQS)
> #define VGIC_MAX_CPUS KVM_MAX_VCPUS
> +#define VGIC_MAX_LRS 64
Consider this instead (for the reason below)
#define VGIC_MAX_LRS (1 << 7)
> /* Sanity checks... */
> #if (VGIC_MAX_CPUS > 8)
> @@ -120,7 +121,7 @@ struct vgic_cpu {
> DECLARE_BITMAP( pending_shared, VGIC_NR_SHARED_IRQS);
>
> /* Bitmap of used/free list registers */
> - DECLARE_BITMAP( lr_used, 64);
> + DECLARE_BITMAP( lr_used, VGIC_MAX_LRS);
>
> /* Number of list registers on this CPU */
> int nr_lr;
> @@ -132,7 +133,7 @@ struct vgic_cpu {
> u32 vgic_eisr[2]; /* Saved only */
> u32 vgic_elrsr[2]; /* Saved only */
> u32 vgic_apr;
> - u32 vgic_lr[64]; /* A15 has only 4... */
> + u32 vgic_lr[VGIC_MAX_LRS];
> #endif
> };
>
> diff --git a/arch/arm/kvm/vgic.c b/arch/arm/kvm/vgic.c
> index a0d283c..90a99fd 100644
> --- a/arch/arm/kvm/vgic.c
> +++ b/arch/arm/kvm/vgic.c
> @@ -1345,6 +1345,8 @@ int kvm_vgic_hyp_init(void)
>
> vgic_nr_lr = readl_relaxed(vgic_vctrl_base + GICH_VTR);
> vgic_nr_lr = (vgic_nr_lr & 0x1f) + 1;
There is a bug here. It should be:
vgic_nr_lr = (vgic_nr_lr & 0x2f) + 1;
> + if (vgic_nr_lr > VGIC_MAX_LRS)
> + vgic_nr_lr = VGIC_MAX_LRS; /* TODO: Clear remaining LRs */
Why? VGIC_MAX_LRS isn't a configurable value, but a maximum value
defined by the specification. This is the maximum you can fit in a 6 bit
field, plus one (1 << 7, exactly).
> ret = create_hyp_io_mappings(vgic_vctrl_base,
> vgic_vctrl_base + resource_size(&vctrl_res),
> --
>
> Thanks,
> -Christoffer
>
--
Jazz is not dead. It just smells funny...
next prev parent reply other threads:[~2013-01-15 11:00 UTC|newest]
Thread overview: 79+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-08 18:41 [PATCH v5 00/12] KVM/ARM vGIC support Christoffer Dall
2013-01-08 18:41 ` [PATCH v5 01/12] KVM: ARM: Introduce KVM_SET_DEVICE_ADDRESS ioctl Christoffer Dall
2013-01-08 22:36 ` Scott Wood
2013-01-08 23:17 ` Christoffer Dall
2013-01-08 23:29 ` Scott Wood
2013-01-08 23:49 ` Christoffer Dall
2013-01-09 0:12 ` Scott Wood
2013-01-09 10:02 ` Alexander Graf
2013-01-09 14:48 ` Peter Maydell
2013-01-09 14:58 ` Alexander Graf
2013-01-09 15:11 ` Peter Maydell
2013-01-09 15:17 ` Christoffer Dall
2013-01-09 15:20 ` Alexander Graf
2013-01-09 15:22 ` Marc Zyngier
2013-01-09 15:28 ` Alexander Graf
2013-01-09 15:50 ` Marc Zyngier
2013-01-09 15:56 ` Alexander Graf
2013-01-09 16:12 ` Marc Zyngier
2013-01-09 16:29 ` Christoffer Dall
2013-01-08 18:41 ` [PATCH v5 02/12] ARM: KVM: Keep track of currently running vcpus Christoffer Dall
2013-01-08 18:41 ` [PATCH v5 03/12] ARM: gic: define GICH offsets for VGIC support Christoffer Dall
2013-01-08 18:41 ` [PATCH v5 04/12] ARM: KVM: Initial VGIC infrastructure code Christoffer Dall
2013-01-14 15:31 ` Will Deacon
2013-01-14 21:08 ` Christoffer Dall
2013-01-14 21:28 ` [kvmarm] " Alexander Graf
2013-01-14 22:50 ` Will Deacon
2013-01-15 10:33 ` Marc Zyngier
2013-01-08 18:41 ` [PATCH v5 05/12] ARM: KVM: VGIC accept vcpu and dist base addresses from user space Christoffer Dall
2013-01-08 18:42 ` [PATCH v5 06/12] ARM: KVM: VGIC distributor handling Christoffer Dall
2013-01-14 15:39 ` Will Deacon
2013-01-14 21:55 ` Christoffer Dall
2013-01-08 18:42 ` [PATCH v5 07/12] ARM: KVM: VGIC virtual CPU interface management Christoffer Dall
2013-01-14 15:42 ` Will Deacon
2013-01-14 22:02 ` Christoffer Dall
2013-01-15 11:00 ` Marc Zyngier [this message]
2013-01-15 14:31 ` Christoffer Dall
2013-01-16 15:29 ` Christoffer Dall
2013-01-16 16:09 ` Marc Zyngier
2013-01-16 16:13 ` Christoffer Dall
2013-01-16 16:17 ` [kvmarm] " Marc Zyngier
2013-01-08 18:42 ` [PATCH v5 08/12] ARM: KVM: vgic: retire queued, disabled interrupts Christoffer Dall
2013-01-08 18:42 ` [PATCH v5 09/12] ARM: KVM: VGIC interrupt injection Christoffer Dall
2013-01-08 18:42 ` [PATCH v5 10/12] ARM: KVM: VGIC control interface world switch Christoffer Dall
2013-01-08 18:42 ` [PATCH v5 11/12] ARM: KVM: VGIC initialisation code Christoffer Dall
2013-01-08 18:42 ` [PATCH v5 12/12] ARM: KVM: Add VGIC configuration option Christoffer Dall
2013-01-09 13:28 ` Sergei Shtylyov
2013-01-09 16:42 ` Christoffer Dall
2013-01-09 16:26 ` [PATCH v5.1 0/2] KVM: ARM: Rename KVM_SET_DEVICE_ADDRESS Christoffer Dall
2013-01-09 16:26 ` [PATCH v5.1 1/2] KVM: ARM: Introduce KVM_SET_DEVICE_ADDRESS ioctl Christoffer Dall
2013-01-09 16:26 ` [PATCH v5.1 2/2] ARM: KVM: VGIC accept vcpu and dist base addresses from user space Christoffer Dall
2013-01-09 16:48 ` [kvmarm] [PATCH v5.1 0/2] KVM: ARM: Rename KVM_SET_DEVICE_ADDRESS Alexander Graf
2013-01-09 19:50 ` Scott Wood
2013-01-09 20:12 ` Alexander Graf
2013-01-09 21:15 ` Scott Wood
2013-01-09 21:37 ` Alexander Graf
2013-01-09 22:10 ` Scott Wood
2013-01-09 22:26 ` Christoffer Dall
2013-01-09 22:34 ` Alexander Graf
2013-01-10 11:15 ` Alexander Graf
2013-01-10 11:18 ` Gleb Natapov
2013-01-09 22:30 ` Alexander Graf
2013-01-10 10:17 ` Peter Maydell
2013-01-10 11:06 ` Alexander Graf
2013-01-10 11:53 ` Marc Zyngier
2013-01-10 11:57 ` Alexander Graf
2013-01-10 22:28 ` Marcelo Tosatti
2013-01-10 22:40 ` Scott Wood
2013-01-11 0:35 ` Marcelo Tosatti
2013-01-11 1:10 ` Scott Wood
2013-01-11 7:26 ` Christoffer Dall
2013-01-11 18:39 ` Marcelo Tosatti
2013-01-11 19:11 ` Alexander Graf
2013-01-11 19:18 ` Marcelo Tosatti
2013-01-11 19:33 ` Christoffer Dall
2013-01-11 15:42 ` Alexander Graf
2013-01-11 20:11 ` Scott Wood
2013-01-11 20:26 ` Alexander Graf
2013-01-11 19:17 ` Alexander Graf
2013-01-10 22:21 ` Marcelo Tosatti
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=50F536DC.2060702@arm.com \
--to=marc.zyngier@arm.com \
--cc=linux-arm-kernel@lists.infradead.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).