Re: [PATCH] KVM: PPC: Book3S HV: XIVE: fix page offset when clearing ESB pages

[Greg Kurz <groug@kaod.org>]

On Tue, 28 May 2019 23:13:24 +0200
Cédric Le Goater <clg@kaod.org> wrote:

> Under XIVE, the ESB pages of an interrupt are used for interrupt
> management (EOI) and triggering. They are made available to guests
> through a mapping of the XIVE KVM device.
> 
> When a device is passed-through, the passthru_irq helpers,
> kvmppc_xive_set_mapped() and kvmppc_xive_clr_mapped(), clear the ESB
> pages of the guest IRQ number being mapped and let the VM fault
> handler repopulate with the correct page.
> 
> The ESB pages are mapped at offset 4 (KVM_XIVE_ESB_PAGE_OFFSET) in the
> KVM device mapping. Unfortunately, this offset was not taken into
> account when clearing the pages. This lead to issues with the

Good catch ! :)

Reviwed-by: Greg Kurz <groug@kaod.org>

> passthrough devices for which the interrupts were not functional under
> some guest configuration (tg3 and single CPU) or in any configuration

And this patch fixes my tg3 use case.

Tested-by: Greg Kurz <groug@kaod.org>

> (e1000e adapter).
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> 
>  if unmap_mapping_pages() could be called from a module, we would
>  simplify a bit this code.
> 
>  arch/powerpc/kvm/book3s_xive_native.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
> index 8b762e3ebbc5..5596c8ec221a 100644
> --- a/arch/powerpc/kvm/book3s_xive_native.c
> +++ b/arch/powerpc/kvm/book3s_xive_native.c
> @@ -172,6 +172,7 @@ int kvmppc_xive_native_connect_vcpu(struct kvm_device *dev,
>  static int kvmppc_xive_native_reset_mapped(struct kvm *kvm, unsigned long irq)
>  {
>  	struct kvmppc_xive *xive = kvm->arch.xive;
> +	pgoff_t esb_pgoff = KVM_XIVE_ESB_PAGE_OFFSET + irq * 2;
>  
>  	if (irq >= KVMPPC_XIVE_NR_IRQS)
>  		return -EINVAL;
> @@ -185,7 +186,7 @@ static int kvmppc_xive_native_reset_mapped(struct kvm *kvm, unsigned long irq)
>  	mutex_lock(&xive->mapping_lock);
>  	if (xive->mapping)
>  		unmap_mapping_range(xive->mapping,
> -				    irq * (2ull << PAGE_SHIFT),
> +				    esb_pgoff << PAGE_SHIFT,
>  				    2ull << PAGE_SHIFT, 1);
>  	mutex_unlock(&xive->mapping_lock);
>  	return 0;