Hi,

On 27/04/2017 13:27, Christoffer Dall wrote:
> On Fri, Apr 14, 2017 at 12:15:20PM +0200, Eric Auger wrote:
>> GITS_CREADR needs to be restored so let's implement the associated
>> uaccess_write_its callback. The write only is allowed if the its
>> is disabled.
>>
>> Signed-off-by: Eric Auger <eric.au...@redhat.com>
>>
>> ---
>> v4 -> v5:
>> - keep Stalled bit
>> - vgic_mmio_uaccess_write_its_creadr can now return an error
>>
>> v3 -> v4:
>> - REGISTER_ITS_DESC_UACCESS now introduced in this patch
>> - we now check the its is disabled
>> ---
>>  virt/kvm/arm/vgic/vgic-its.c | 42 ++++++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 40 insertions(+), 2 deletions(-)
>>
>> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
>> index a9a2c12..79ed1c2 100644
>> --- a/virt/kvm/arm/vgic/vgic-its.c
>> +++ b/virt/kvm/arm/vgic/vgic-its.c
>> @@ -1213,6 +1213,33 @@ static unsigned long vgic_mmio_read_its_creadr(struct 
>> kvm *kvm,
>>      return extract_bytes(its->creadr, addr & 0x7, len);
>>  }
>>  
>> +static int vgic_mmio_uaccess_write_its_creadr(struct kvm *kvm,
>> +                                          struct vgic_its *its,
>> +                                          gpa_t addr, unsigned int len,
>> +                                          unsigned long val)
>> +{
>> +    int ret = 0;
>> +    u32 reg;
>> +
>> +    mutex_lock(&its->cmd_lock);
>> +
>> +    if (its->enabled) {
>> +            ret = -EBUSY;
>> +            goto out;
>> +    }
>> +
>> +    reg = update_64bit_reg(its->creadr, addr & 7, len, val);
> 
> you theoretically don't need this, since you prevent 32-bit accesses to
> this register, but I guess it doesn't hurt...
OK
> 
>> +    if (ITS_CMD_OFFSET(reg) >= ITS_CMD_BUFFER_SIZE(its->cbaser)) {
>> +            ret = -EINVAL;
>> +            goto out;
>> +    }
> 
> can the creadr value be unaligned to the command size?  I don't think
> you check that anywhere here?
makes sense. I will add this check.

Thanks!

Eric
> 
> Thanks,
> -Christoffer
> 
>> +
>> +    its->creadr = reg;
>> +out:
>> +    mutex_unlock(&its->cmd_lock);
>> +    return ret;
>> +}
>> +
>>  #define BASER_INDEX(addr) (((addr) / sizeof(u64)) & 0x7)
>>  static unsigned long vgic_mmio_read_its_baser(struct kvm *kvm,
>>                                            struct vgic_its *its,
>> @@ -1317,6 +1344,16 @@ static void vgic_mmio_write_its_ctlr(struct kvm *kvm, 
>> struct vgic_its *its,
>>      .its_write = wr,                                        \
>>  }
>>  
>> +#define REGISTER_ITS_DESC_UACCESS(off, rd, wr, uwr, length, acc)\
>> +{                                                           \
>> +    .reg_offset = off,                                      \
>> +    .len = length,                                          \
>> +    .access_flags = acc,                                    \
>> +    .its_read = rd,                                         \
>> +    .its_write = wr,                                        \
>> +    .uaccess_its_write = uwr,                               \
>> +}
>> +
>>  static void its_mmio_write_wi(struct kvm *kvm, struct vgic_its *its,
>>                            gpa_t addr, unsigned int len, unsigned long val)
>>  {
>> @@ -1339,8 +1376,9 @@ static struct vgic_register_region its_registers[] = {
>>      REGISTER_ITS_DESC(GITS_CWRITER,
>>              vgic_mmio_read_its_cwriter, vgic_mmio_write_its_cwriter, 8,
>>              VGIC_ACCESS_64bit | VGIC_ACCESS_32bit),
>> -    REGISTER_ITS_DESC(GITS_CREADR,
>> -            vgic_mmio_read_its_creadr, its_mmio_write_wi, 8,
>> +    REGISTER_ITS_DESC_UACCESS(GITS_CREADR,
>> +            vgic_mmio_read_its_creadr, its_mmio_write_wi,
>> +            vgic_mmio_uaccess_write_its_creadr, 8,
>>              VGIC_ACCESS_64bit | VGIC_ACCESS_32bit),
>>      REGISTER_ITS_DESC(GITS_BASER,
>>              vgic_mmio_read_its_baser, vgic_mmio_write_its_baser, 0x40,
>> -- 
>> 2.5.5
>>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Reply via email to