Re: [PATCH v4 07/22] KVM: arm64: ITS: Implement vgic_mmio_uaccess_write_its_creadr

Marc Zyngier
2017-04-08T10:36:06Z

On Mon, Mar 27 2017 at 10:30:57 AM, Eric Auger <eric.auger@redhat.com> wrote:
> GITS_CREADR needs to be restored so let's implement the associated
> uaccess_write_its callback. The write only is allowed if the its
> is disabled.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>
> ---
>
> v3 -> v4:
> - REGISTER_ITS_DESC_UACCESS now introduced in this patch
> - we now check the its is disabled
> ---
> virt/kvm/arm/vgic/vgic-its.c | 37 +++++++++++++++++++++++++++++++++++--
> 1 file changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
> index 9b9ea86..a5f3abe 100644
> --- a/virt/kvm/arm/vgic/vgic-its.c
> +++ b/virt/kvm/arm/vgic/vgic-its.c
> @@ -1213,6 +1213,28 @@ static unsigned long vgic_mmio_read_its_creadr(struct kvm *kvm,
> return extract_bytes(its->creadr, addr & 0x7, len);
> }
>
> +static void vgic_mmio_uaccess_write_its_creadr(struct kvm *kvm,
> + struct vgic_its *its,
> + gpa_t addr, unsigned int len,
> + unsigned long val)
> +{
> + u32 reg;
> +
> + mutex_lock(&its->cmd_lock);
> +
> + if (its->enabled)
> + goto out;
> +
> + reg = update_64bit_reg(its->creadr, addr & 7, len, val);
> + reg = ITS_CMD_OFFSET(reg);
> + if (reg >= ITS_CMD_BUFFER_SIZE(its->cbaser))
> + goto out;
> +
> + its->creadr = reg;

Careful. Here, you're loosing the "Stalled" bit. We do not implement
support for it yet, but I wonder if we should preserve it. If not, we
should at least document that we're purposely stripping that bit.

> +out:
> + mutex_unlock(&its->cmd_lock);
> +}
> +
> #define BASER_INDEX(addr) (((addr) / sizeof(u64)) & 0x7)
> static unsigned long vgic_mmio_read_its_baser(struct kvm *kvm,
> struct vgic_its *its,
> @@ -1317,6 +1339,16 @@ static void vgic_mmio_write_its_ctlr(struct kvm *kvm, struct vgic_its *its,
> .its_write = wr, \
> }
>
> +#define REGISTER_ITS_DESC_UACCESS(off, rd, wr, uwr, length, acc)\
> +{ \
> + .reg_offset = off, \
> + .len = length, \
> + .access_flags = acc, \
> + .its_read = rd, \
> + .its_write = wr, \
> + .uaccess_its_write = uwr, \
> +}
> +
> static void its_mmio_write_wi(struct kvm *kvm, struct vgic_its *its,
> gpa_t addr, unsigned int len, unsigned long val)
> {
> @@ -1339,8 +1371,9 @@ static struct vgic_register_region its_registers[] = {
> REGISTER_ITS_DESC(GITS_CWRITER,
> vgic_mmio_read_its_cwriter, vgic_mmio_write_its_cwriter, 8,
> VGIC_ACCESS_64bit | VGIC_ACCESS_32bit),
> - REGISTER_ITS_DESC(GITS_CREADR,
> - vgic_mmio_read_its_creadr, its_mmio_write_wi, 8,
> + REGISTER_ITS_DESC_UACCESS(GITS_CREADR,
> + vgic_mmio_read_its_creadr, its_mmio_write_wi,
> + vgic_mmio_uaccess_write_its_creadr, 8,
> VGIC_ACCESS_64bit | VGIC_ACCESS_32bit),
> REGISTER_ITS_DESC(GITS_BASER,
> vgic_mmio_read_its_baser, vgic_mmio_write_its_baser, 0x40,

Thanks,

M.
--
Jazz is not dead. It just smells funny.