Skip to content

Commit

Permalink
KVM: arm/arm64: vgic: Permit uaccess writes to return errors
Browse files Browse the repository at this point in the history 
Currently we do not allow any vgic mmio write operations to fail, which
makes sense from mmio traps from the guest.  However, we should be able
to report failures to userspace, if userspace writes incompatible values
to read-only registers.  Rework the internal interface to allow errors
to be returned on the write side for userspace writes.

Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Christoffer Dall <christoffer.dall@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
  • Loading branch information
Christoffer Dall authored and Marc Zyngier committed Jul 21, 2018
1 parent 8732209 commit c6e0917
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 18 deletions.
12 changes: 7 additions & 5 deletions virt/kvm/arm/vgic/vgic-mmio-v3.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,9 +249,9 @@ static unsigned long vgic_v3_uaccess_read_pending(struct kvm_vcpu *vcpu,
return value;
}

static void vgic_v3_uaccess_write_pending(struct kvm_vcpu *vcpu,
gpa_t addr, unsigned int len,
unsigned long val)
static int vgic_v3_uaccess_write_pending(struct kvm_vcpu *vcpu,
gpa_t addr, unsigned int len,
unsigned long val)
{
u32 intid = VGIC_ADDR_TO_INTID(addr, 1);
int i;
Expand All @@ -276,6 +276,8 @@ static void vgic_v3_uaccess_write_pending(struct kvm_vcpu *vcpu,

vgic_put_irq(vcpu->kvm, irq);
}

return 0;
}

/* We want to avoid outer shareable. */
Expand Down Expand Up @@ -468,7 +470,7 @@ static const struct vgic_register_region vgic_v3_dist_registers[] = {
VGIC_ACCESS_32bit),
REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ICPENDR,
vgic_mmio_read_pending, vgic_mmio_write_cpending,
vgic_mmio_read_raz, vgic_mmio_write_wi, 1,
vgic_mmio_read_raz, vgic_mmio_uaccess_write_wi, 1,
VGIC_ACCESS_32bit),
REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ISACTIVER,
vgic_mmio_read_active, vgic_mmio_write_sactive,
Expand Down Expand Up @@ -541,7 +543,7 @@ static const struct vgic_register_region vgic_v3_sgibase_registers[] = {
VGIC_ACCESS_32bit),
REGISTER_DESC_WITH_LENGTH_UACCESS(GICR_ICPENDR0,
vgic_mmio_read_pending, vgic_mmio_write_cpending,
vgic_mmio_read_raz, vgic_mmio_write_wi, 4,
vgic_mmio_read_raz, vgic_mmio_uaccess_write_wi, 4,
VGIC_ACCESS_32bit),
REGISTER_DESC_WITH_LENGTH_UACCESS(GICR_ISACTIVER0,
vgic_mmio_read_active, vgic_mmio_write_sactive,
Expand Down
18 changes: 13 additions & 5 deletions virt/kvm/arm/vgic/vgic-mmio.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,13 @@ void vgic_mmio_write_wi(struct kvm_vcpu *vcpu, gpa_t addr,
/* Ignore */
}

int vgic_mmio_uaccess_write_wi(struct kvm_vcpu *vcpu, gpa_t addr,
unsigned int len, unsigned long val)
{
/* Ignore */
return 0;
}

/*
* Read accesses to both GICD_ICENABLER and GICD_ISENABLER return the value
* of the enabled bit, so there is only one function for both here.
Expand Down Expand Up @@ -363,11 +370,12 @@ void vgic_mmio_write_cactive(struct kvm_vcpu *vcpu,
mutex_unlock(&vcpu->kvm->lock);
}

void vgic_mmio_uaccess_write_cactive(struct kvm_vcpu *vcpu,
int vgic_mmio_uaccess_write_cactive(struct kvm_vcpu *vcpu,
gpa_t addr, unsigned int len,
unsigned long val)
{
__vgic_mmio_write_cactive(vcpu, addr, len, val);
return 0;
}

static void __vgic_mmio_write_sactive(struct kvm_vcpu *vcpu,
Expand Down Expand Up @@ -399,11 +407,12 @@ void vgic_mmio_write_sactive(struct kvm_vcpu *vcpu,
mutex_unlock(&vcpu->kvm->lock);
}

void vgic_mmio_uaccess_write_sactive(struct kvm_vcpu *vcpu,
int vgic_mmio_uaccess_write_sactive(struct kvm_vcpu *vcpu,
gpa_t addr, unsigned int len,
unsigned long val)
{
__vgic_mmio_write_sactive(vcpu, addr, len, val);
return 0;
}

unsigned long vgic_mmio_read_priority(struct kvm_vcpu *vcpu,
Expand Down Expand Up @@ -735,10 +744,9 @@ static int vgic_uaccess_write(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,

r_vcpu = iodev->redist_vcpu ? iodev->redist_vcpu : vcpu;
if (region->uaccess_write)
region->uaccess_write(r_vcpu, addr, sizeof(u32), *val);
else
region->write(r_vcpu, addr, sizeof(u32), *val);
return region->uaccess_write(r_vcpu, addr, sizeof(u32), *val);

region->write(r_vcpu, addr, sizeof(u32), *val);
return 0;
}

Expand Down
19 changes: 11 additions & 8 deletions virt/kvm/arm/vgic/vgic-mmio.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ struct vgic_register_region {
unsigned long (*uaccess_read)(struct kvm_vcpu *vcpu, gpa_t addr,
unsigned int len);
union {
void (*uaccess_write)(struct kvm_vcpu *vcpu, gpa_t addr,
unsigned int len, unsigned long val);
int (*uaccess_write)(struct kvm_vcpu *vcpu, gpa_t addr,
unsigned int len, unsigned long val);
int (*uaccess_its_write)(struct kvm *kvm, struct vgic_its *its,
gpa_t addr, unsigned int len,
unsigned long val);
Expand Down Expand Up @@ -134,6 +134,9 @@ unsigned long vgic_mmio_read_rao(struct kvm_vcpu *vcpu,
void vgic_mmio_write_wi(struct kvm_vcpu *vcpu, gpa_t addr,
unsigned int len, unsigned long val);

int vgic_mmio_uaccess_write_wi(struct kvm_vcpu *vcpu, gpa_t addr,
unsigned int len, unsigned long val);

unsigned long vgic_mmio_read_enable(struct kvm_vcpu *vcpu,
gpa_t addr, unsigned int len);

Expand Down Expand Up @@ -167,13 +170,13 @@ void vgic_mmio_write_sactive(struct kvm_vcpu *vcpu,
gpa_t addr, unsigned int len,
unsigned long val);

void vgic_mmio_uaccess_write_cactive(struct kvm_vcpu *vcpu,
gpa_t addr, unsigned int len,
unsigned long val);
int vgic_mmio_uaccess_write_cactive(struct kvm_vcpu *vcpu,
gpa_t addr, unsigned int len,
unsigned long val);

void vgic_mmio_uaccess_write_sactive(struct kvm_vcpu *vcpu,
gpa_t addr, unsigned int len,
unsigned long val);
int vgic_mmio_uaccess_write_sactive(struct kvm_vcpu *vcpu,
gpa_t addr, unsigned int len,
unsigned long val);

unsigned long vgic_mmio_read_priority(struct kvm_vcpu *vcpu,
gpa_t addr, unsigned int len);
Expand Down

0 comments on commit c6e0917

Please sign in to comment.