On 25.09.2013, at 23:30, Christoffer Dall wrote:

> On Sun, Aug 25, 2013 at 04:24:20PM +0100, Alexander Graf wrote:
>> 
>> On 23.08.2013, at 20:20, Christoffer Dall wrote:
>> 
>>> Implement support for the CPU interface register access driven by MMIO
>>> address offsets from the CPU interface base address.  Useful for user
>>> space to support save/restore of the VGIC state.
>>> 
>>> This commit adds support only for the same logic as the current VGIC
>>> support, and no more.  For example, the active priority registers are
>>> handled as RAZ/WI, just like setting priorities on the emulated
>>> distributor.
>>> 
>>> Signed-off-by: Christoffer Dall <[email protected]>
>>> ---
>>> virt/kvm/arm/vgic.c |   66 
>>> +++++++++++++++++++++++++++++++++++++++++++++++----
>>> 1 file changed, 62 insertions(+), 4 deletions(-)
>>> 
>>> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
>>> index d44b5a1..257dbae 100644
>>> --- a/virt/kvm/arm/vgic.c
>>> +++ b/virt/kvm/arm/vgic.c
>>> @@ -1684,9 +1684,67 @@ int kvm_vgic_addr(struct kvm *kvm, unsigned long 
>>> type, u64 *addr, bool write)
>>> static bool handle_cpu_mmio_misc(struct kvm_vcpu *vcpu,
>>>                              struct kvm_exit_mmio *mmio, phys_addr_t offset)
>>> {
>>> -   return true;
>>> +   struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
>>> +   u32 reg, mask = 0, shift = 0;
>>> +   bool updated = false;
>>> +
>>> +   switch (offset & ~0x3) {
>>> +   case GIC_CPU_CTRL:
>>> +           mask = GICH_VMCR_CTRL_MASK;
>>> +           shift = GICH_VMCR_CTRL_SHIFT;
>>> +           break;
>>> +   case GIC_CPU_PRIMASK:
>>> +           mask = GICH_VMCR_PRIMASK_MASK;
>>> +           shift = GICH_VMCR_PRIMASK_SHIFT;
>>> +           break;
>>> +   case GIC_CPU_BINPOINT:
>>> +           mask = GICH_VMCR_BINPOINT_MASK;
>>> +           shift = GICH_VMCR_BINPOINT_SHIFT;
>>> +           break;
>>> +   case GIC_CPU_ALIAS_BINPOINT:
>>> +           mask = GICH_VMCR_ALIAS_BINPOINT_MASK;
>>> +           shift = GICH_VMCR_ALIAS_BINPOINT_SHIFT;
>>> +           break;
>>> +   }
>>> +
>>> +   if (!mmio->is_write) {
>>> +           reg = (vgic_cpu->vgic_vmcr & mask) >> shift;
>>> +           memcpy(mmio->data, &reg, sizeof(reg));
>>> +   } else {
>>> +           memcpy(&reg, mmio->data, sizeof(reg));
>>> +           reg = (reg << shift) & mask;
>>> +           if (reg != (vgic_cpu->vgic_vmcr & mask))
>>> +                   updated = true;
>>> +           vgic_cpu->vgic_vmcr &= ~mask;
>>> +           vgic_cpu->vgic_vmcr |= reg;
>>> +   }
>>> +   return updated;
>>> +}
>>> +
>>> +static bool handle_mmio_abpr(struct kvm_vcpu *vcpu,
>>> +                        struct kvm_exit_mmio *mmio, phys_addr_t offset)
>>> +{
>>> +   return handle_cpu_mmio_misc(vcpu, mmio, GIC_CPU_ALIAS_BINPOINT);
>>> +}
>>> +
>>> +static bool handle_cpu_mmio_ident(struct kvm_vcpu *vcpu,
>>> +                             struct kvm_exit_mmio *mmio,
>>> +                             phys_addr_t offset)
>>> +{
>>> +   u32 reg;
>>> +
>>> +   if (mmio->is_write)
>>> +           return false;
>>> +
>>> +   reg = 0x0002043B;
>> 
>> This wants a comment and probably also a #define :).
>> 
> 
> Marc, where does the 0x4b0 product id code come from for the distributor
> IIDR?
> 
> Would this be satisfying?
> 
> 
> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
> index 5214424..558be38 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -71,6 +71,9 @@
> #define VGIC_ADDR_UNDEF               (-1)
> #define IS_VGIC_ADDR_UNDEF(_x)  ((_x) == VGIC_ADDR_UNDEF)
> 
> +#define GIC_PRODUCT_ID               0x4b0

This is a specific GIC version. PL390 for example is 0x3b0:

  
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0416b/Beiggeba.html

That should be reflected in the #define. If it means "GICv2" then it should be 
GICV2_PRODUCT_ID for example.

> +#define ARM_JEP106_IMPLEMENTER       0x43b

I think naming this JEP106_IMPLEMENTER_ARM makes it more obvious that we're 
talking about the implementer code for ARM. Or maybe just only IMPLEMENTER_ARM.

> +
> /* Physical address of vgic virtual cpu interface */
> static phys_addr_t vgic_vcpu_base;
> 
> @@ -331,7 +334,7 @@ static bool handle_mmio_misc(struct kvm_vcpu *vcpu,
>               break;
> 
>       case 8:                 /* IIDR */
> -             reg = 0x4B00043B;
> +             reg = (GIC_PRODUCT_ID << 20) | ARM_JEP106_IMPLEMENTER;
>               vgic_reg_access(mmio, &reg, word_offset,
>                               ACCESS_READ_VALUE | ACCESS_WRITE_IGNORED);
>               break;
> @@ -1734,7 +1737,7 @@ static bool handle_cpu_mmio_ident(struct kvm_vcpu *vcpu,
>       if (mmio->is_write)
>               return false;
> 
> -     reg = 0x0002043B;
> +     reg = (GIC_PRODUCT_ID << 20) | (0x2 << 16) | ARM_JEP106_IMPLEMENTER;

What is the 0x2 here?


Alex

>       mmio_data_write(mmio, ~0, reg);
>       return false;
> }
> 
> Thanks,
> -Christoffer

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to