Hi,
On 9/29/21 11:17 PM, Ricardo Koller wrote:
> On Wed, Sep 29, 2021 at 06:29:21PM +0200, Eric Auger wrote:
>> Hi Ricardo,
>>
>> On 9/28/21 8:47 PM, Ricardo Koller wrote:
>>> Add the new vgic_check_iorange helper that checks that an iorange is
>>> sane: the start address and size have valid alignments, the range is
>>> within the addressable PA range, start+size doesn't overflow, and the
>>> start wasn't already defined.
>>>
>>> No functional change.
>>>
>>> Signed-off-by: Ricardo Koller <ricarkol@xxxxxxxxxx>
>>> ---
>>> arch/arm64/kvm/vgic/vgic-kvm-device.c | 22 ++++++++++++++++++++++
>>> arch/arm64/kvm/vgic/vgic.h | 4 ++++
>>> 2 files changed, 26 insertions(+)
>>>
>>> diff --git a/arch/arm64/kvm/vgic/vgic-kvm-device.c b/arch/arm64/kvm/vgic/vgic-kvm-device.c
>>> index 7740995de982..f714aded67b2 100644
>>> --- a/arch/arm64/kvm/vgic/vgic-kvm-device.c
>>> +++ b/arch/arm64/kvm/vgic/vgic-kvm-device.c
>>> @@ -29,6 +29,28 @@ int vgic_check_ioaddr(struct kvm *kvm, phys_addr_t *ioaddr,
>>> return 0;
>>> }
>>>
>>> +int vgic_check_iorange(struct kvm *kvm, phys_addr_t *ioaddr,
>>> + phys_addr_t addr, phys_addr_t alignment,
>>> + phys_addr_t size)
>>> +{
>>> + int ret;
>>> +
>>> + ret = vgic_check_ioaddr(kvm, ioaddr, addr, alignment);
>> nit: not related to this patch but I am just wondering why we are
>> passing phys_addr_t *ioaddr downto vgic_check_ioaddr and thus to
>>
>> vgic_check_iorange()? This must be a leftover of some old code?
>>
> It's used to check that the base of a region is not already set.
> kvm_vgic_addr() uses it to make that check;
> vgic_v3_alloc_redist_region() does not:
>
> rdreg->base = VGIC_ADDR_UNDEF; // so the "not already defined" check passes
> ret = vgic_check_ioaddr(kvm, &rdreg->base, base, SZ_64K);
Yes but I meant why a pointer?
Eric
>
> Thanks,
> Ricardo
>
>>> + if (ret)
>>> + return ret;
>>> +
>>> + if (!IS_ALIGNED(size, alignment))
>>> + return -EINVAL;
>>> +
>>> + if (addr + size < addr)
>>> + return -EINVAL;
>>> +
>>> + if (addr + size > kvm_phys_size(kvm))
>>> + return -E2BIG;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> static int vgic_check_type(struct kvm *kvm, int type_needed)
>>> {
>>> if (kvm->arch.vgic.vgic_model != type_needed)
>>> diff --git a/arch/arm64/kvm/vgic/vgic.h b/arch/arm64/kvm/vgic/vgic.h
>>> index 14a9218641f5..c4df4dcef31f 100644
>>> --- a/arch/arm64/kvm/vgic/vgic.h
>>> +++ b/arch/arm64/kvm/vgic/vgic.h
>>> @@ -175,6 +175,10 @@ void vgic_irq_handle_resampling(struct vgic_irq *irq,
>>> int vgic_check_ioaddr(struct kvm *kvm, phys_addr_t *ioaddr,
>>> phys_addr_t addr, phys_addr_t alignment);
>>>
>>> +int vgic_check_iorange(struct kvm *kvm, phys_addr_t *ioaddr,
>>> + phys_addr_t addr, phys_addr_t alignment,
>>> + phys_addr_t size);
>>> +
>>> void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu);
>>> void vgic_v2_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr);
>>> void vgic_v2_clear_lr(struct kvm_vcpu *vcpu, int lr);
>> Besides
>> Reviewed-by: Eric Auger <eric.auger@xxxxxxxxxx>
>> Eric
>>
