On Fri, 04 Sep 2020 17:00:36 +0100,
Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> wrote:
>
> On Thu, 3 Sep 2020 16:26:09 +0100
> Marc Zyngier <maz@xxxxxxxxxx> wrote:
[...]
> > +static int rvic_set_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
> > +{
> > + struct rvic_vm_data *data;
> > + struct kvm_vcpu *vcpu;
> > + u32 __user *uaddr, val;
> > + u16 trusted, total;
> > + int i, ret = -ENXIO;
> > +
> > + mutex_lock(&dev->kvm->lock);
> > +
> > + switch (attr->group) {
> > + case KVM_DEV_ARM_RVIC_GRP_NR_IRQS:
> > + if (attr->attr)
> > + break;
> > +
> > + if (dev->kvm->arch.irqchip_data) {
> > + ret = -EBUSY;
> > + break;
> > + }
> > +
> > + uaddr = (u32 __user *)(uintptr_t)attr->addr;
> > + if (get_user(val, uaddr)) {
> > + ret = -EFAULT;
> > + break;
> > + }
> > +
> > + trusted = FIELD_GET(KVM_DEV_ARM_RVIC_GRP_NR_TRUSTED_MASK, val);
> > + total = FIELD_GET(KVM_DEV_ARM_RVIC_GRP_NR_TOTAL_MASK, val);
> > + if (total < trusted || trusted < 32 || total < 64 ||
> > + trusted % 32 || total % 32 || total > 2048) {
>
> As I read the spec, we need at least 32 untrusted. (R0058)
> This condition seems to allow that if trusted = 64 and untrusted = 0
Well spotted. I think the following would capture the constraints
correctly:
if (total <= trusted || trusted < 32 || total < 64 ||
trusted % 32 || total % 32 || total > 2048) {
On the other hand, I wonder if this code would gain from being
directly written in terms of trusted/untrusted, rather than
trusted/total. It could make the reading against the spec easier.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
