Re: [PATCH 22/23] KVM: arm64: Add a rVIC/rVID in-kernel implementation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux