RE: [PATCH v3 2/5] KVM: arm64: Implement vGICv3 distributor and redistributor access from userspace

[Pavel Fedin <p.fedin@xxxxxxxxxxx>]

 Hello!

> > +	mmio.is_write = is_write;
> > +
> > +	if (attr->attr & KVM_DEV_ARM_VGIC_64BIT) {
> > +		u64 __user *uaddr = (u64 __user *)(long)attr->addr;
> > +		__le64 data;
> > +
> > +		if (is_write) {
> > +			u64 reg;
> > +
> > +			if (get_user(reg, uaddr))
> 
> Wouldn't the use of copy_from_user/copy_to_user here make this whole
> switch redundant? Even if not, you could think about moving this logic
> into into vgic_attr_regs_access() and just hardcode "len=4" into GICv2
> accesses.

 No, because endianess conversion is still strongly typed. And this ends up in:

if (is_write) {
	u64 reg;
	__le64 data;

	copy_from_user(&reg, uaddr, mmio.len);

	if (mmio.len == 8)
		data = cpu_to_le64(reg);
	else
		*((__le32 *)&data) = cpu_to_le32(*((__le32 *)&reg);

isn't this even more ugly? Regardless of where it is placed.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


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