On Mon, Jan 07, 2019 at 07:43:18PM +0100, Cédric Le Goater wrote:
> This will let the guest create a memory mapping to expose the ESB MMIO
> regions used to control the interrupt sources, to trigger events, to
> EOI or to turn off the sources.
>
> Signed-off-by: Cédric Le Goater <clg@xxxxxxxx>
> ---
> arch/powerpc/include/uapi/asm/kvm.h | 4 ++
> arch/powerpc/kvm/book3s_xive_native.c | 97 +++++++++++++++++++++++++++
> 2 files changed, 101 insertions(+)
>
> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
> index 8c876c166ef2..6bb61ba141c2 100644
> --- a/arch/powerpc/include/uapi/asm/kvm.h
> +++ b/arch/powerpc/include/uapi/asm/kvm.h
> @@ -675,4 +675,8 @@ struct kvm_ppc_cpu_char {
> #define KVM_XICS_PRESENTED (1ULL << 43)
> #define KVM_XICS_QUEUED (1ULL << 44)
>
> +/* POWER9 XIVE Native Interrupt Controller */
> +#define KVM_DEV_XIVE_GRP_CTRL 1
> +#define KVM_DEV_XIVE_GET_ESB_FD 1
> +
> #endif /* __LINUX_KVM_POWERPC_H */
> diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
> index 115143e76c45..e20081f0c8d4 100644
> --- a/arch/powerpc/kvm/book3s_xive_native.c
> +++ b/arch/powerpc/kvm/book3s_xive_native.c
> @@ -153,6 +153,85 @@ int kvmppc_xive_native_connect_vcpu(struct kvm_device *dev,
> return rc;
> }
>
> +static int xive_native_esb_fault(struct vm_fault *vmf)
> +{
> + struct vm_area_struct *vma = vmf->vma;
> + struct kvmppc_xive *xive = vma->vm_file->private_data;
> + struct kvmppc_xive_src_block *sb;
> + struct kvmppc_xive_irq_state *state;
> + struct xive_irq_data *xd;
> + u32 hw_num;
> + u16 src;
> + u64 page;
> + unsigned long irq;
> +
> + /*
> + * Linux/KVM uses a two pages ESB setting, one for trigger and
> + * one for EOI
> + */
> + irq = vmf->pgoff / 2;
> +
> + sb = kvmppc_xive_find_source(xive, irq, &src);
> + if (!sb) {
> + pr_err("%s: source %lx not found !\n", __func__, irq);
In general it's a bad idea to have a printk that userspace can trigger
at will without any rate-limiting. Is there a real reason why this
printk is needed (and can't be pr_devel)?
> + return VM_FAULT_SIGBUS;
> + }
> +
> + state = &sb->irq_state[src];
> + kvmppc_xive_select_irq(state, &hw_num, &xd);
> +
> + arch_spin_lock(&sb->lock);
> +
> + /*
> + * first/even page is for trigger
> + * second/odd page is for EOI and management.
> + */
> + page = vmf->pgoff % 2 ? xd->eoi_page : xd->trig_page;
> + arch_spin_unlock(&sb->lock);
> +
> + if (!page) {
> + pr_err("%s: acessing invalid ESB page for source %lx !\n",
> + __func__, irq);
Does this represent a exceptional condition that userspace can't just
trigger at will (i.e. it implies the presence of a kernel bug)? If
not then the same comment as above applies.
Paul.
