From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-07 19:05:44
Hello,
On the POWER9 processor, the XIVE interrupt controller can control
interrupt sources using MMIO to trigger events, to EOI or to turn off
the sources. Priority management and interrupt acknowledgment is also
controlled by MMIO in the CPU presenter subengine.
PowerNV/baremetal Linux runs natively under XIVE but sPAPR guests need
special support from the hypervisor to do the same. This is called the
XIVE native exploitation mode and today, it can be activated under the
PowerPC Hypervisor, pHyp. However, Linux/KVM lacks XIVE native support
and still offers the old interrupt mode interface using a
XICS-over-XIVE glue which implements the XICS hcalls.
The following series is proposal to add the same support under KVM.
A new KVM device is introduced for the XIVE native exploitation
mode. It reuses most of the XICS-over-XIVE glue implementation
structures which are internal to KVM but has a completely different
interface. A set of Hypervisor calls configures the sources and the
event queues and from there, all control is done by the guest through
MMIOs.
These MMIO regions (ESB and TIMA) are exposed to guests in QEMU,
similarly to VFIO, and the associated VMAs are populated dynamically
with the appropriate pages using a fault handler. This is implemented
with a couple of KVM device ioctls.
On a POWER9 sPAPR machine, the Client Architecture Support (CAS)
negotiation process determines whether the guest operates with a
interrupt controller using the XICS legacy model, as found on POWER8,
or in XIVE exploitation mode. Which means that the KVM interrupt
device should be created at runtime, after the machine as started.
This requires extra KVM support to create/destroy KVM devices. The
last patches are an attempt to solve that problem.
Migration has its own specific needs. The patchset provides the
necessary routines to quiesce XIVE, to capture and restore the state
of the different structures used by KVM, OPAL and HW. Extra OPAL
support is required for these.
GitHub trees available here :
QEMU sPAPR:
https://github.com/legoater/qemu/commits/xive-next
Linux/KVM:
https://github.com/legoater/linux/commits/xive-5.0
OPAL:
https://github.com/legoater/skiboot/commits/xive
Best wishes for 2019 !
C.
Cédric Le Goater (19):
powerpc/xive: export flags for the XIVE native exploitation mode
hcalls
powerpc/xive: add OPAL extensions for the XIVE native exploitation
support
KVM: PPC: Book3S HV: check the IRQ controller type
KVM: PPC: Book3S HV: export services for the XIVE native exploitation
device
KVM: PPC: Book3S HV: add a new KVM device for the XIVE native
exploitation mode
KVM: PPC: Book3S HV: add a GET_ESB_FD control to the XIVE native
device
KVM: PPC: Book3S HV: add a GET_TIMA_FD control to XIVE native device
KVM: PPC: Book3S HV: add a VC_BASE control to the XIVE native device
KVM: PPC: Book3S HV: add a SET_SOURCE control to the XIVE native
device
KVM: PPC: Book3S HV: add a EISN attribute to kvmppc_xive_irq_state
KVM: PPC: Book3S HV: add support for the XIVE native exploitation mode
hcalls
KVM: PPC: Book3S HV: record guest queue page address
KVM: PPC: Book3S HV: add a SYNC control for the XIVE native migration
KVM: PPC: Book3S HV: add a control to make the XIVE EQ pages dirty
KVM: PPC: Book3S HV: add get/set accessors for the source
configuration
KVM: PPC: Book3S HV: add get/set accessors for the EQ configuration
KVM: PPC: Book3S HV: add get/set accessors for the VP XIVE state
KVM: PPC: Book3S HV: add passthrough support
KVM: introduce a KVM_DELETE_DEVICE ioctl
arch/powerpc/include/asm/kvm_host.h | 2 +
arch/powerpc/include/asm/kvm_ppc.h | 69 +
arch/powerpc/include/asm/opal-api.h | 11 +-
arch/powerpc/include/asm/opal.h | 7 +
arch/powerpc/include/asm/xive.h | 40 +
arch/powerpc/include/uapi/asm/kvm.h | 47 +
arch/powerpc/kvm/book3s_xive.h | 82 +
include/linux/kvm_host.h | 2 +
include/uapi/linux/kvm.h | 5 +
arch/powerpc/kvm/book3s.c | 31 +-
arch/powerpc/kvm/book3s_hv.c | 29 +
arch/powerpc/kvm/book3s_hv_builtin.c | 196 +++
arch/powerpc/kvm/book3s_hv_rm_xive_native.c | 47 +
arch/powerpc/kvm/book3s_xive.c | 149 +-
arch/powerpc/kvm/book3s_xive_native.c | 1406 +++++++++++++++++
.../powerpc/kvm/book3s_xive_native_template.c | 398 +++++
arch/powerpc/kvm/powerpc.c | 30 +
arch/powerpc/sysdev/xive/native.c | 110 ++
arch/powerpc/sysdev/xive/spapr.c | 28 +-
virt/kvm/kvm_main.c | 39 +
arch/powerpc/kvm/Makefile | 4 +-
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 52 +
.../powerpc/platforms/powernv/opal-wrappers.S | 3 +
23 files changed, 2722 insertions(+), 65 deletions(-)
create mode 100644 arch/powerpc/kvm/book3s_hv_rm_xive_native.c
create mode 100644 arch/powerpc/kvm/book3s_xive_native.c
create mode 100644 arch/powerpc/kvm/book3s_xive_native_template.c
--
2.20.1
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-07 18:53:33
The XIVE native exploitation mode specs define a set of Hypervisor
calls to configure the sources and the event queues :
- H_INT_GET_SOURCE_INFO
used to obtain the address of the MMIO page of the Event State
Buffer (PQ bits) entry associated with the source.
- H_INT_SET_SOURCE_CONFIG
assigns a source to a "target".
- H_INT_GET_SOURCE_CONFIG
determines which "target" and "priority" is assigned to a source
- H_INT_GET_QUEUE_INFO
returns the address of the notification management page associated
with the specified "target" and "priority".
- H_INT_SET_QUEUE_CONFIG
sets or resets the event queue for a given "target" and "priority".
It is also used to set the notification configuration associated
with the queue, only unconditional notification is supported for
the moment. Reset is performed with a queue size of 0 and queueing
is disabled in that case.
- H_INT_GET_QUEUE_CONFIG
returns the queue settings for a given "target" and "priority".
- H_INT_RESET
resets all of the guest's internal interrupt structures to their
initial state, losing all configuration set via the hcalls
H_INT_SET_SOURCE_CONFIG and H_INT_SET_QUEUE_CONFIG.
- H_INT_SYNC
issue a synchronisation on a source to make sure all notifications
have reached their queue.
Calls that still need to be addressed :
H_INT_SET_OS_REPORTING_LINE
H_INT_GET_OS_REPORTING_LINE
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/include/asm/kvm_ppc.h | 43 ++
arch/powerpc/kvm/book3s_xive.h | 54 +++
arch/powerpc/kvm/book3s_hv.c | 29 ++
arch/powerpc/kvm/book3s_hv_builtin.c | 196 +++++++++
arch/powerpc/kvm/book3s_hv_rm_xive_native.c | 47 +++
arch/powerpc/kvm/book3s_xive_native.c | 326 ++++++++++++++-
.../powerpc/kvm/book3s_xive_native_template.c | 371 ++++++++++++++++++
arch/powerpc/kvm/Makefile | 2 +
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 52 +++
9 files changed, 1118 insertions(+), 2 deletions(-)
create mode 100644 arch/powerpc/kvm/book3s_hv_rm_xive_native.c
@@ -532,6 +596,248 @@ static int kvmppc_xive_native_create(struct kvm_device *dev, u32 type)returnret;}+staticintkvmppc_h_int_set_source_config(structkvm_vcpu*vcpu,+unsignedlongflags,+unsignedlongirq,+unsignedlongserver,+unsignedlongpriority,+unsignedlongeisn)+{+structkvmppc_xive*xive=vcpu->kvm->arch.xive;+structkvmppc_xive_src_block*sb;+structkvmppc_xive_irq_state*state;+intrc=0;+u16idx;++pr_devel("H_INT_SET_SOURCE_CONFIG flags=%08lx irq=%lx server=%ld priority=%ld eisn=%lx\n",+flags,irq,server,priority,eisn);++if(flags&~(XIVE_SPAPR_SRC_SET_EISN|XIVE_SPAPR_SRC_MASK))+returnH_PARAMETER;++sb=kvmppc_xive_find_source(xive,irq,&idx);+if(!sb)+returnH_P2;+state=&sb->irq_state[idx];++if(!(flags&XIVE_SPAPR_SRC_SET_EISN))+eisn=state->eisn;++if(priority!=xive_prio_from_guest(priority)){+pr_err("invalid priority for queue %ld for VCPU %ld\n",+priority,server);+returnH_P3;+}++/* TODO: handle XIVE_SPAPR_SRC_MASK */++rc=kvmppc_xive_native_set_source_config(xive,sb,state,server,+priority,eisn);+if(!rc)+returnH_SUCCESS;+elseif(rc==-EINVAL)+returnH_P4;/* no server found */+else+returnH_HARDWARE;+}++staticintkvmppc_h_int_set_queue_config(structkvm_vcpu*vcpu,+unsignedlongflags,+unsignedlongserver,+unsignedlongpriority,+unsignedlongqpage,+unsignedlongqsize)+{+structkvm*kvm=vcpu->kvm;+structkvmppc_xive_vcpu*xc=vcpu->arch.xive_vcpu;+structxive_q*q;+intrc;+__be32*qaddr=0;+structpage*page;++pr_devel("H_INT_SET_QUEUE_CONFIG flags=%08lx server=%ld priority=%ld qpage=%08lx qsize=%ld\n",+flags,server,priority,qpage,qsize);++if(flags&~XIVE_SPAPR_EQ_ALWAYS_NOTIFY)+returnH_PARAMETER;++if(xc->server_num!=server){+vcpu=kvmppc_xive_find_server(kvm,server);+if(!vcpu){+pr_debug("Can't find server %ld\n",server);+returnH_P2;+}+xc=vcpu->arch.xive_vcpu;+}++if(priority!=xive_prio_from_guest(priority)||priority==MASKED){+pr_err("invalid priority for queue %ld for VCPU %d\n",+priority,xc->server_num);+returnH_P3;+}+q=&xc->queues[priority];++rc=xive_native_validate_queue_size(qsize);+if(rc){+pr_err("invalid queue size %ld\n",qsize);+returnH_P5;+}++/* reset queue and disable queueing */+if(!qsize){+rc=xive_native_configure_queue(xc->vp_id,q,priority,+NULL,0,true);+if(rc){+pr_err("Failed to reset queue %ld for VCPU %d: %d\n",+priority,xc->server_num,rc);+returnH_HARDWARE;+}++if(q->qpage){+put_page(virt_to_page(q->qpage));+q->qpage=NULL;+}++returnH_SUCCESS;+}++page=gfn_to_page(kvm,gpa_to_gfn(qpage));+if(is_error_page(page)){+pr_warn("Couldn't get guest page for %lx!\n",qpage);+returnH_P4;+}+qaddr=page_to_virt(page)+(qpage&~PAGE_MASK);++rc=xive_native_configure_queue(xc->vp_id,q,priority,+(__be32*)qaddr,qsize,true);+if(rc){+pr_err("Failed to configure queue %ld for VCPU %d: %d\n",+priority,xc->server_num,rc);+put_page(page);+returnH_HARDWARE;+}++rc=kvmppc_xive_attach_escalation(vcpu,priority);+if(rc){+xive_native_cleanup_queue(vcpu,priority);+returnH_HARDWARE;+}++returnH_SUCCESS;+}++staticvoidkvmppc_xive_reset_sources(structkvmppc_xive_src_block*sb)+{+inti;++for(i=0;i<KVMPPC_XICS_IRQ_PER_ICS;i++){+structkvmppc_xive_irq_state*state=&sb->irq_state[i];++if(!state->valid)+continue;++if(state->act_priority==MASKED)+continue;++arch_spin_lock(&sb->lock);+state->eisn=0;+state->act_server=0;+state->act_priority=MASKED;+xive_vm_esb_load(&state->ipi_data,XIVE_ESB_SET_PQ_01);+xive_native_configure_irq(state->ipi_number,0,MASKED,0);+if(state->pt_number){+xive_vm_esb_load(state->pt_data,XIVE_ESB_SET_PQ_01);+xive_native_configure_irq(state->pt_number,+0,MASKED,0);+}+arch_spin_unlock(&sb->lock);+}+}++staticintkvmppc_h_int_reset(structkvmppc_xive*xive,unsignedlongflags)+{+structkvm*kvm=xive->kvm;+structkvm_vcpu*vcpu;+unsignedinti;++pr_devel("H_INT_RESET flags=%08lx\n",flags);++if(flags)+returnH_PARAMETER;++mutex_lock(&kvm->lock);++kvm_for_each_vcpu(i,vcpu,kvm){+structkvmppc_xive_vcpu*xc=vcpu->arch.xive_vcpu;+unsignedintprio;++if(!xc)+continue;++kvmppc_xive_disable_vcpu_interrupts(vcpu);++for(prio=0;prio<KVMPPC_XIVE_Q_COUNT;prio++){++if(xc->esc_virq[prio]){+free_irq(xc->esc_virq[prio],vcpu);+irq_dispose_mapping(xc->esc_virq[prio]);+kfree(xc->esc_virq_names[prio]);+xc->esc_virq[prio]=0;+}++xive_native_cleanup_queue(vcpu,prio);+}+}++for(i=0;i<=xive->max_sbid;i++){+if(xive->src_blocks[i])+kvmppc_xive_reset_sources(xive->src_blocks[i]);+}++mutex_unlock(&kvm->lock);++returnH_SUCCESS;+}++intkvmppc_xive_native_hcall(structkvm_vcpu*vcpu,u32req)+{+structkvmppc_xive*xive=vcpu->kvm->arch.xive;+intrc;++if(!xive||!vcpu->arch.xive_vcpu)+returnH_FUNCTION;++switch(req){+caseH_INT_SET_QUEUE_CONFIG:+rc=kvmppc_h_int_set_queue_config(vcpu,+kvmppc_get_gpr(vcpu,4),+kvmppc_get_gpr(vcpu,5),+kvmppc_get_gpr(vcpu,6),+kvmppc_get_gpr(vcpu,7),+kvmppc_get_gpr(vcpu,8));+break;++caseH_INT_SET_SOURCE_CONFIG:+rc=kvmppc_h_int_set_source_config(vcpu,+kvmppc_get_gpr(vcpu,4),+kvmppc_get_gpr(vcpu,5),+kvmppc_get_gpr(vcpu,6),+kvmppc_get_gpr(vcpu,7),+kvmppc_get_gpr(vcpu,8));+break;++caseH_INT_RESET:+rc=kvmppc_h_int_reset(xive,kvmppc_get_gpr(vcpu,4));+break;++default:+rc=H_NOT_AVAILABLE;+}++returnrc;+}+EXPORT_SYMBOL_GPL(kvmppc_xive_native_hcall);+staticintxive_native_debug_show(structseq_file*m,void*private){structkvmppc_xive*xive=m->private;
@@ -8,6 +8,279 @@#define XGLUE(a, b) a##b#define GLUE(a, b) XGLUE(a, b)+X_STATICintGLUE(X_PFX,h_int_get_source_info)(structkvm_vcpu*vcpu,+unsignedlongflags,+unsignedlongirq)+{+structkvmppc_xive*xive=vcpu->kvm->arch.xive;+structkvmppc_xive_src_block*sb;+structkvmppc_xive_irq_state*state;+structxive_irq_data*xd;+u32hw_num;+u16src;+unsignedlongesb_addr;++pr_devel("H_INT_GET_SOURCE_INFO flags=%08lx irq=%lx\n",flags,irq);++if(!xive)+returnH_FUNCTION;++if(flags)+returnH_PARAMETER;++sb=kvmppc_xive_find_source(xive,irq,&src);+if(!sb){+pr_debug("source %lx not found !\n",irq);+returnH_P2;+}+state=&sb->irq_state[src];++arch_spin_lock(&sb->lock);+kvmppc_xive_select_irq(state,&hw_num,&xd);++vcpu->arch.regs.gpr[4]=0;+if(xd->flags&XIVE_IRQ_FLAG_STORE_EOI)+vcpu->arch.regs.gpr[4]|=XIVE_SPAPR_SRC_STORE_EOI;++/*+*ForcetheuseoftheH_INT_ESBhcallincaseofaVirtual+*LSIinterrupt.ThisisnecessaryunderKVMtore-trigger+*theinterruptifthelevelisstillasserted+*/+if(state->lsi){+vcpu->arch.regs.gpr[4]|=XIVE_SPAPR_SRC_LSI;+vcpu->arch.regs.gpr[4]|=XIVE_SPAPR_SRC_H_INT_ESB;+}++/*+*Linux/KVMusesatwopagesESBsetting,onefortriggerand+*oneforEOI+*/+esb_addr=xive->vc_base+(irq<<(PAGE_SHIFT+1));++/* EOI/management page is the second/odd page */+if(xd->eoi_page&&+!(vcpu->arch.regs.gpr[4]&XIVE_SPAPR_SRC_H_INT_ESB))+vcpu->arch.regs.gpr[5]=esb_addr+(1ull<<PAGE_SHIFT);+else+vcpu->arch.regs.gpr[5]=-1;++/* Trigger page is always the first/even page */+if(xd->trig_page)+vcpu->arch.regs.gpr[6]=esb_addr;+else+vcpu->arch.regs.gpr[6]=-1;++vcpu->arch.regs.gpr[7]=PAGE_SHIFT;+arch_spin_unlock(&sb->lock);+returnH_SUCCESS;+}++X_STATICintGLUE(X_PFX,h_int_get_source_config)(structkvm_vcpu*vcpu,+unsignedlongflags,+unsignedlongirq)+{+structkvmppc_xive*xive=vcpu->kvm->arch.xive;+structkvmppc_xive_src_block*sb;+structkvmppc_xive_irq_state*state;+u16src;++pr_devel("H_INT_GET_SOURCE_CONFIG flags=%08lx irq=%lx\n",flags,irq);++if(!xive)+returnH_FUNCTION;++if(flags)+returnH_PARAMETER;++sb=kvmppc_xive_find_source(xive,irq,&src);+if(!sb){+pr_debug("source %lx not found !\n",irq);+returnH_P2;+}+state=&sb->irq_state[src];++arch_spin_lock(&sb->lock);+vcpu->arch.regs.gpr[4]=state->act_server;+vcpu->arch.regs.gpr[5]=state->act_priority;+vcpu->arch.regs.gpr[6]=state->number;+arch_spin_unlock(&sb->lock);++returnH_SUCCESS;+}++X_STATICintGLUE(X_PFX,h_int_get_queue_info)(structkvm_vcpu*vcpu,+unsignedlongflags,+unsignedlongserver,+unsignedlongpriority)+{+structkvmppc_xive*xive=vcpu->kvm->arch.xive;+structkvmppc_xive_vcpu*xc=vcpu->arch.xive_vcpu;+structxive_q*q;++pr_devel("H_INT_GET_QUEUE_INFO flags=%08lx server=%ld priority=%ld\n",+flags,server,priority);++if(!xive)+returnH_FUNCTION;++if(flags)+returnH_PARAMETER;++if(xc->server_num!=server){+structkvm_vcpu*vc;++vc=kvmppc_xive_find_server(vcpu->kvm,server);+if(!vc){+pr_debug("server %ld not found\n",server);+returnH_P2;+}+xc=vc->arch.xive_vcpu;+}++if(priority!=xive_prio_from_guest(priority)||priority==MASKED){+pr_debug("invalid priority for queue %ld for VCPU %ld\n",+priority,server);+returnH_P3;+}+q=&xc->queues[priority];++vcpu->arch.regs.gpr[4]=q->eoi_phys;+/* TODO: Power of 2 page size of the notification page */+vcpu->arch.regs.gpr[5]=0;+returnH_SUCCESS;+}++X_STATICintGLUE(X_PFX,get_queue_state)(structkvm_vcpu*vcpu,+structkvmppc_xive_vcpu*xc,+unsignedlongprio)+{+intrc;+u32qtoggle;+u32qindex;++rc=xive_native_get_queue_state(xc->vp_id,prio,&qtoggle,&qindex);+if(rc)+returnrc;++vcpu->arch.regs.gpr[4]|=((unsignedlong)qtoggle)<<62;+vcpu->arch.regs.gpr[7]=qindex;+return0;+}++X_STATICintGLUE(X_PFX,h_int_get_queue_config)(structkvm_vcpu*vcpu,+unsignedlongflags,+unsignedlongserver,+unsignedlongpriority)+{+structkvmppc_xive*xive=vcpu->kvm->arch.xive;+structkvmppc_xive_vcpu*xc=vcpu->arch.xive_vcpu;+structxive_q*q;+u64qpage;+u64qsize;+u64qeoi_page;+u32escalate_irq;+u64qflags;+intrc;++pr_devel("H_INT_GET_QUEUE_CONFIG flags=%08lx server=%ld priority=%ld\n",+flags,server,priority);++if(!xive)+returnH_FUNCTION;++if(flags&~XIVE_SPAPR_EQ_DEBUG)+returnH_PARAMETER;++if(xc->server_num!=server){+structkvm_vcpu*vc;++vc=kvmppc_xive_find_server(vcpu->kvm,server);+if(!vc){+pr_debug("server %ld not found\n",server);+returnH_P2;+}+xc=vc->arch.xive_vcpu;+}++if(priority!=xive_prio_from_guest(priority)||priority==MASKED){+pr_debug("invalid priority for queue %ld for VCPU %ld\n",+priority,server);+returnH_P3;+}+q=&xc->queues[priority];++rc=xive_native_get_queue_info(xc->vp_id,priority,&qpage,&qsize,+&qeoi_page,&escalate_irq,&qflags);+if(rc)+returnH_HARDWARE;++vcpu->arch.regs.gpr[4]=0;+if(qflags&OPAL_XIVE_EQ_ALWAYS_NOTIFY)+vcpu->arch.regs.gpr[4]|=XIVE_SPAPR_EQ_ALWAYS_NOTIFY;++vcpu->arch.regs.gpr[5]=qpage;+vcpu->arch.regs.gpr[6]=qsize;+if(flags&XIVE_SPAPR_EQ_DEBUG){+rc=GLUE(X_PFX,get_queue_state)(vcpu,xc,priority);+if(rc)+returnH_HARDWARE;+}+returnH_SUCCESS;+}++/* TODO H_INT_SET_OS_REPORTING_LINE */+X_STATICintGLUE(X_PFX,h_int_set_os_reporting_line)(structkvm_vcpu*vcpu,+unsignedlongflags,+unsignedlongline)+{+structkvmppc_xive*xive=vcpu->kvm->arch.xive;++pr_devel("H_INT_SET_OS_REPORTING_LINE flags=%08lx line=%ld\n",+flags,line);++if(!xive)+returnH_FUNCTION;++if(flags)+returnH_PARAMETER;++returnH_FUNCTION;+}++/* TODO H_INT_GET_OS_REPORTING_LINE*/+X_STATICintGLUE(X_PFX,h_int_get_os_reporting_line)(structkvm_vcpu*vcpu,+unsignedlongflags,+unsignedlongserver,+unsignedlongline)+{+structkvmppc_xive*xive=vcpu->kvm->arch.xive;+structkvmppc_xive_vcpu*xc=vcpu->arch.xive_vcpu;++pr_devel("H_INT_GET_OS_REPORTING_LINE flags=%08lx server=%ld line=%ld\n",+flags,server,line);++if(!xive)+returnH_FUNCTION;++if(flags)+returnH_PARAMETER;++if(xc->server_num!=server){+structkvm_vcpu*vc;++vc=kvmppc_xive_find_server(vcpu->kvm,server);+if(!vc){+pr_debug("server %ld not found\n",server);+returnH_P2;+}+xc=vc->arch.xive_vcpu;+}++returnH_FUNCTION;++}+/**TODO:introduceacommontemplatefilewiththeXIVEnativelayer*andtheXICS-on-XIVEgluefortheutilityfunctions
@@ -25,3 +298,101 @@ static u8 GLUE(X_PFX, esb_load)(struct xive_irq_data *xd, u32 offset)#endifreturn(u8)val;}++staticu8GLUE(X_PFX,esb_store)(structxive_irq_data*xd,u32offset,u64data)+{+u64val;++if(xd->flags&XIVE_IRQ_FLAG_SHIFT_BUG)+offset|=offset<<4;++val=__x_readq(__x_eoi_page(xd)+offset);+#ifdef __LITTLE_ENDIAN__+val>>=64-8;+#endif+return(u8)val;+}++X_STATICintGLUE(X_PFX,h_int_esb)(structkvm_vcpu*vcpu,unsignedlongflags,+unsignedlongirq,unsignedlongoffset,+unsignedlongdata)+{+structkvmppc_xive*xive=vcpu->kvm->arch.xive;+structkvmppc_xive_src_block*sb;+structkvmppc_xive_irq_state*state;+structxive_irq_data*xd;+u32hw_num;+u16src;++if(!xive)+returnH_FUNCTION;++if(flags)+returnH_PARAMETER;++sb=kvmppc_xive_find_source(xive,irq,&src);+if(!sb){+pr_debug("source %lx not found !\n",irq);+returnH_P2;+}+state=&sb->irq_state[src];++if(offset>(1ull<<PAGE_SHIFT))+returnH_P3;++arch_spin_lock(&sb->lock);+kvmppc_xive_select_irq(state,&hw_num,&xd);++if(flags&XIVE_SPAPR_ESB_STORE){+GLUE(X_PFX,esb_store)(xd,offset,data);+vcpu->arch.regs.gpr[4]=-1;+}else{+/* Virtual LSI EOI handling */+if(state->lsi&&offset==XIVE_ESB_LOAD_EOI){+GLUE(X_PFX,esb_load)(xd,XIVE_ESB_SET_PQ_00);+if(state->asserted&&__x_trig_page(xd))+__x_writeq(0,__x_trig_page(xd));+vcpu->arch.regs.gpr[4]=0;+}else{+vcpu->arch.regs.gpr[4]=+GLUE(X_PFX,esb_load)(xd,offset);+}+}+arch_spin_unlock(&sb->lock);++returnH_SUCCESS;+}++X_STATICintGLUE(X_PFX,h_int_sync)(structkvm_vcpu*vcpu,unsignedlongflags,+unsignedlongirq)+{+structkvmppc_xive*xive=vcpu->kvm->arch.xive;+structkvmppc_xive_src_block*sb;+structkvmppc_xive_irq_state*state;+structxive_irq_data*xd;+u32hw_num;+u16src;++pr_devel("H_INT_SYNC flags=%08lx irq=%lx\n",flags,irq);++if(!xive)+returnH_FUNCTION;++if(flags)+returnH_PARAMETER;++sb=kvmppc_xive_find_source(xive,irq,&src);+if(!sb){+pr_debug("source %lx not found !\n",irq);+returnH_P2;+}+state=&sb->irq_state[src];++arch_spin_lock(&sb->lock);++kvmppc_xive_select_irq(state,&hw_num,&xd);+xive_native_sync_source(hw_num);++arch_spin_unlock(&sb->lock);+returnH_SUCCESS;+}
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-07 18:55:50
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@kaod.org>
---
arch/powerpc/include/uapi/asm/kvm.h | 4 ++
arch/powerpc/kvm/book3s_xive_native.c | 97 +++++++++++++++++++++++++++
2 files changed, 101 insertions(+)
@@ -153,6 +153,85 @@ int kvmppc_xive_native_connect_vcpu(struct kvm_device *dev,returnrc;}+staticintxive_native_esb_fault(structvm_fault*vmf)+{+structvm_area_struct*vma=vmf->vma;+structkvmppc_xive*xive=vma->vm_file->private_data;+structkvmppc_xive_src_block*sb;+structkvmppc_xive_irq_state*state;+structxive_irq_data*xd;+u32hw_num;+u16src;+u64page;+unsignedlongirq;++/*+*Linux/KVMusesatwopagesESBsetting,onefortriggerand+*oneforEOI+*/+irq=vmf->pgoff/2;++sb=kvmppc_xive_find_source(xive,irq,&src);+if(!sb){+pr_err("%s: source %lx not found !\n",__func__,irq);+returnVM_FAULT_SIGBUS;+}++state=&sb->irq_state[src];+kvmppc_xive_select_irq(state,&hw_num,&xd);++arch_spin_lock(&sb->lock);++/*+*first/evenpageisfortrigger+*second/oddpageisforEOIandmanagement.+*/+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);+returnVM_FAULT_SIGBUS;+}++vmf_insert_pfn(vma,vmf->address,page>>PAGE_SHIFT);+returnVM_FAULT_NOPAGE;+}++staticconststructvm_operations_structxive_native_esb_vmops={+.fault=xive_native_esb_fault,+};++staticintxive_native_esb_mmap(structfile*file,structvm_area_struct*vma)+{+/* There are two ESB pages (trigger and EOI) per IRQ */+if(vma_pages(vma)+vma->vm_pgoff>KVMPPC_XIVE_NR_IRQS*2)+return-EINVAL;++vma->vm_flags|=VM_IO|VM_PFNMAP;+vma->vm_page_prot=pgprot_noncached(vma->vm_page_prot);+vma->vm_ops=&xive_native_esb_vmops;+return0;+}++staticconststructfile_operationsxive_native_esb_fops={+.mmap=xive_native_esb_mmap,+};++staticintkvmppc_xive_native_get_esb_fd(structkvmppc_xive*xive,u64addr)+{+u64__user*ubufp=(u64__user*)addr;+intret;++ret=anon_inode_getfd("[xive-esb]",&xive_native_esb_fops,xive,+O_RDWR|O_CLOEXEC);+if(ret<0)+returnret;++returnput_user(ret,ubufp);+}+staticintkvmppc_xive_native_set_attr(structkvm_device*dev,structkvm_device_attr*attr){
@@ -162,12 +241,30 @@ static int kvmppc_xive_native_set_attr(struct kvm_device *dev,staticintkvmppc_xive_native_get_attr(structkvm_device*dev,structkvm_device_attr*attr){+structkvmppc_xive*xive=dev->private;++switch(attr->group){+caseKVM_DEV_XIVE_GRP_CTRL:+switch(attr->attr){+caseKVM_DEV_XIVE_GET_ESB_FD:+returnkvmppc_xive_native_get_esb_fd(xive,attr->addr);+}+break;+}return-ENXIO;}staticintkvmppc_xive_native_has_attr(structkvm_device*dev,structkvm_device_attr*attr){+switch(attr->group){+caseKVM_DEV_XIVE_GRP_CTRL:+switch(attr->attr){+caseKVM_DEV_XIVE_GET_ESB_FD:+return0;+}+break;+}return-ENXIO;}
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-07 18:57:20
The guest physical address of the event queue will be part of the
state to transfer in the migration. Cache its value when the queue is
configured, it will save us an OPAL call.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/include/asm/xive.h | 2 ++
arch/powerpc/kvm/book3s_xive_native.c | 4 ++++
2 files changed, 6 insertions(+)
@@ -74,6 +74,8 @@ struct xive_q {u32esc_irq;atomic_tcount;atomic_tpending_count;+u64guest_qpage;+u32guest_qsize;};/* Global enable flags for the XIVE support */
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-07 18:59:32
We will have different KVM devices for interrupts, one for the
XICS-over-XIVE mode and one for the XIVE native exploitation
mode. Let's add some checks to make sure we are not mixing the
interfaces in KVM.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/kvm/book3s_xive.c | 6 ++++++
1 file changed, 6 insertions(+)
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-07 19:02:29
These flags are shared between Linux/KVM implementing the hypervisor
calls for the XIVE native exploitation mode and the driver for the
sPAPR guests.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/include/asm/xive.h | 23 +++++++++++++++++++++++
arch/powerpc/sysdev/xive/spapr.c | 28 ++++++++--------------------
2 files changed, 31 insertions(+), 20 deletions(-)
@@ -432,7 +420,7 @@ static int xive_spapr_configure_queue(u32 target, struct xive_q *q, u8 prio,q->eoi_phys=esn_page;/* Default is to always notify */-flags=XIVE_EQ_ALWAYS_NOTIFY;+flags=XIVE_SPAPR_EQ_ALWAYS_NOTIFY;/* Configure and enable the queue in HW */rc=plpar_int_set_queue_config(flags,target,prio,qpage_phys,order);
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-07 19:04:13
When the VM is stopped in a migration sequence, the sources are masked
and the XIVE IC is synced to stabilize the EQs. When done, the KVM
ioctl KVM_DEV_XIVE_SAVE_EQ_PAGES is called to mark dirty the EQ pages.
The migration can then transfer the remaining dirty pages to the
destination and start collecting the state of the devices.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/include/uapi/asm/kvm.h | 1 +
arch/powerpc/kvm/book3s_xive_native.c | 40 +++++++++++++++++++++++++++
2 files changed, 41 insertions(+)
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-07 19:07:29
This is the basic framework for the new KVM device supporting the XIVE
native exploitation mode. The user interface exposes a new capability
and a new KVM device to be used by QEMU.
Internally, the interface to the new KVM device is protected with a
new interrupt mode: KVMPPC_IRQ_XIVE.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/include/asm/kvm_host.h | 2 +
arch/powerpc/include/asm/kvm_ppc.h | 21 ++
arch/powerpc/kvm/book3s_xive.h | 3 +
include/uapi/linux/kvm.h | 3 +
arch/powerpc/kvm/book3s.c | 7 +-
arch/powerpc/kvm/book3s_xive_native.c | 332 ++++++++++++++++++++++++++
arch/powerpc/kvm/powerpc.c | 30 +++
arch/powerpc/kvm/Makefile | 2 +-
8 files changed, 398 insertions(+), 2 deletions(-)
create mode 100644 arch/powerpc/kvm/book3s_xive_native.c
@@ -0,0 +1,332 @@+// SPDX-License-Identifier: GPL-2.0+/*+*Copyright(c)2017-2019,IBMCorporation.+*/++#define pr_fmt(fmt) "xive-kvm: " fmt++#include<linux/anon_inodes.h>+#include<linux/kernel.h>+#include<linux/kvm_host.h>+#include<linux/err.h>+#include<linux/gfp.h>+#include<linux/spinlock.h>+#include<linux/delay.h>+#include<linux/percpu.h>+#include<linux/cpumask.h>+#include<asm/uaccess.h>+#include<asm/kvm_book3s.h>+#include<asm/kvm_ppc.h>+#include<asm/hvcall.h>+#include<asm/xics.h>+#include<asm/xive.h>+#include<asm/xive-regs.h>+#include<asm/debug.h>+#include<asm/debugfs.h>+#include<asm/time.h>+#include<asm/opal.h>++#include<linux/debugfs.h>+#include<linux/seq_file.h>++#include"book3s_xive.h"++staticvoidxive_native_cleanup_queue(structkvm_vcpu*vcpu,intprio)+{+structkvmppc_xive_vcpu*xc=vcpu->arch.xive_vcpu;+structxive_q*q=&xc->queues[prio];++xive_native_disable_queue(xc->vp_id,q,prio);+if(q->qpage){+put_page(virt_to_page(q->qpage));+q->qpage=NULL;+}+}++voidkvmppc_xive_native_cleanup_vcpu(structkvm_vcpu*vcpu)+{+structkvmppc_xive_vcpu*xc=vcpu->arch.xive_vcpu;+inti;++if(!kvmppc_xive_enabled(vcpu))+return;++if(!xc)+return;++pr_devel("native_cleanup_vcpu(cpu=%d)\n",xc->server_num);++/* Ensure no interrupt is still routed to that VP */+xc->valid=false;+kvmppc_xive_disable_vcpu_interrupts(vcpu);++/* Disable the VP */+xive_native_disable_vp(xc->vp_id);++/* Free the queues & associated interrupts */+for(i=0;i<KVMPPC_XIVE_Q_COUNT;i++){+/* Free the escalation irq */+if(xc->esc_virq[i]){+free_irq(xc->esc_virq[i],vcpu);+irq_dispose_mapping(xc->esc_virq[i]);+kfree(xc->esc_virq_names[i]);+xc->esc_virq[i]=0;+}++/* Free the queue */+xive_native_cleanup_queue(vcpu,i);+}++/* Free the VP */+kfree(xc);++/* Cleanup the vcpu */+vcpu->arch.irq_type=KVMPPC_IRQ_DEFAULT;+vcpu->arch.xive_vcpu=NULL;+}++intkvmppc_xive_native_connect_vcpu(structkvm_device*dev,+structkvm_vcpu*vcpu,u32cpu)+{+structkvmppc_xive*xive=dev->private;+structkvmppc_xive_vcpu*xc;+intrc;++pr_devel("native_connect_vcpu(cpu=%d)\n",cpu);++if(dev->ops!=&kvm_xive_native_ops){+pr_devel("Wrong ops !\n");+return-EPERM;+}+if(xive->kvm!=vcpu->kvm)+return-EPERM;+if(vcpu->arch.irq_type)+return-EBUSY;+if(kvmppc_xive_find_server(vcpu->kvm,cpu)){+pr_devel("Duplicate !\n");+return-EEXIST;+}+if(cpu>=KVM_MAX_VCPUS){+pr_devel("Out of bounds !\n");+return-EINVAL;+}+xc=kzalloc(sizeof(*xc),GFP_KERNEL);+if(!xc)+return-ENOMEM;++mutex_lock(&vcpu->kvm->lock);+vcpu->arch.xive_vcpu=xc;+xc->xive=xive;+xc->vcpu=vcpu;+xc->server_num=cpu;+xc->vp_id=xive->vp_base+cpu;+xc->valid=true;++rc=xive_native_get_vp_info(xc->vp_id,&xc->vp_cam,&xc->vp_chip_id);+if(rc){+pr_err("Failed to get VP info from OPAL: %d\n",rc);+gotobail;+}++/*+*EnabletheVPfirstasthesingleescalationmodewill+*affectescalationinterruptsnumbering+*/+rc=xive_native_enable_vp(xc->vp_id,xive->single_escalation);+if(rc){+pr_err("Failed to enable VP in OPAL: %d\n",rc);+gotobail;+}++/* Configure VCPU fields for use by assembly push/pull */+vcpu->arch.xive_saved_state.w01=cpu_to_be64(0xff000000);+vcpu->arch.xive_cam_word=cpu_to_be32(xc->vp_cam|TM_QW1W2_VO);++/* TODO: initialize queues ? */++bail:+vcpu->arch.irq_type=KVMPPC_IRQ_XIVE;+mutex_unlock(&vcpu->kvm->lock);+if(rc)+kvmppc_xive_native_cleanup_vcpu(vcpu);++returnrc;+}++staticintkvmppc_xive_native_set_attr(structkvm_device*dev,+structkvm_device_attr*attr)+{+return-ENXIO;+}++staticintkvmppc_xive_native_get_attr(structkvm_device*dev,+structkvm_device_attr*attr)+{+return-ENXIO;+}++staticintkvmppc_xive_native_has_attr(structkvm_device*dev,+structkvm_device_attr*attr)+{+return-ENXIO;+}++staticvoidkvmppc_xive_native_free(structkvm_device*dev)+{+structkvmppc_xive*xive=dev->private;+structkvm*kvm=xive->kvm;+inti;++debugfs_remove(xive->dentry);++pr_devel("Destroying xive native for partition\n");++if(kvm)+kvm->arch.xive=NULL;++/* Mask and free interrupts */+for(i=0;i<=xive->max_sbid;i++){+if(xive->src_blocks[i])+kvmppc_xive_free_sources(xive->src_blocks[i]);+kfree(xive->src_blocks[i]);+xive->src_blocks[i]=NULL;+}++if(xive->vp_base!=XIVE_INVALID_VP)+xive_native_free_vp_block(xive->vp_base);++kfree(xive);+kfree(dev);+}++staticintkvmppc_xive_native_create(structkvm_device*dev,u32type)+{+structkvmppc_xive*xive;+structkvm*kvm=dev->kvm;+intret=0;++pr_devel("Creating xive native for partition\n");++if(kvm->arch.xive)+return-EEXIST;++xive=kzalloc(sizeof(*xive),GFP_KERNEL);+if(!xive)+return-ENOMEM;++dev->private=xive;+xive->dev=dev;+xive->kvm=kvm;+kvm->arch.xive=xive;++/* We use the default queue size set by the host */+xive->q_order=xive_native_default_eq_shift();+if(xive->q_order<PAGE_SHIFT)+xive->q_page_order=0;+else+xive->q_page_order=xive->q_order-PAGE_SHIFT;++/* Allocate a bunch of VPs */+xive->vp_base=xive_native_alloc_vp_block(KVM_MAX_VCPUS);+pr_devel("VP_Base=%x\n",xive->vp_base);++if(xive->vp_base==XIVE_INVALID_VP)+ret=-ENOMEM;++xive->single_escalation=xive_native_has_single_escalation();++if(ret)+kfree(xive);++returnret;+}++staticintxive_native_debug_show(structseq_file*m,void*private)+{+structkvmppc_xive*xive=m->private;+structkvm*kvm=xive->kvm;+structkvm_vcpu*vcpu;+unsignedinti;++if(!kvm)+return0;++seq_puts(m,"=========\nVCPU state\n=========\n");++kvm_for_each_vcpu(i,vcpu,kvm){+structkvmppc_xive_vcpu*xc=vcpu->arch.xive_vcpu;++if(!xc)+continue;++seq_printf(m,"cpu server %#x NSR=%02x CPPR=%02x IBP=%02x PIPR=%02x w01=%016llx w2=%08x\n",+xc->server_num,+vcpu->arch.xive_saved_state.nsr,+vcpu->arch.xive_saved_state.cppr,+vcpu->arch.xive_saved_state.ipb,+vcpu->arch.xive_saved_state.pipr,+vcpu->arch.xive_saved_state.w01,+(u32)vcpu->arch.xive_cam_word);++kvmppc_xive_debug_show_queues(m,vcpu);+}++return0;+}++staticintxive_native_debug_open(structinode*inode,structfile*file)+{+returnsingle_open(file,xive_native_debug_show,inode->i_private);+}++staticconststructfile_operationsxive_native_debug_fops={+.open=xive_native_debug_open,+.read=seq_read,+.llseek=seq_lseek,+.release=single_release,+};++staticvoidxive_native_debugfs_init(structkvmppc_xive*xive)+{+char*name;++name=kasprintf(GFP_KERNEL,"kvm-xive-%p",xive);+if(!name){+pr_err("%s: no memory for name\n",__func__);+return;+}++xive->dentry=debugfs_create_file(name,0444,powerpc_debugfs_root,+xive,&xive_native_debug_fops);++pr_debug("%s: created %s\n",__func__,name);+kfree(name);+}++staticvoidkvmppc_xive_native_init(structkvm_device*dev)+{+structkvmppc_xive*xive=(structkvmppc_xive*)dev->private;++/* Register some debug interfaces */+xive_native_debugfs_init(xive);+}++structkvm_device_opskvm_xive_native_ops={+.name="kvm-xive-native",+.create=kvmppc_xive_native_create,+.init=kvmppc_xive_native_init,+.destroy=kvmppc_xive_native_free,+.set_attr=kvmppc_xive_native_set_attr,+.get_attr=kvmppc_xive_native_get_attr,+.has_attr=kvmppc_xive_native_has_attr,+};++voidkvmppc_xive_native_init_module(void)+{+;+}++voidkvmppc_xive_native_exit_module(void)+{+;+}
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-07 19:08:53
Theses are use to capure the XIVE EAS table of the KVM device, the
configuration of the source targets.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/include/uapi/asm/kvm.h | 11 ++++
arch/powerpc/kvm/book3s_xive_native.c | 87 +++++++++++++++++++++++++++
2 files changed, 98 insertions(+)
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-07 19:10:16
The KVM device for the XIVE native exploitation mode will reuse the
structures of the XICS-over-XIVE glue implementation. Some code will
also be shared : source block creation and destruction, target
selection and escalation attachment.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/kvm/book3s_xive.h | 11 +++++
arch/powerpc/kvm/book3s_xive.c | 89 +++++++++++++++++++---------------
2 files changed, 62 insertions(+), 38 deletions(-)
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-07 19:14:56
Clear the ESB pages from the VMA of the IRQ being pass through to the
guest and let the fault handler repopulate the VMA when the ESB pages
are accessed for an EOI or for a trigger.
Storing the VMA under the KVM XIVE device is a little ugly.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/kvm/book3s_xive.h | 8 +++++++
arch/powerpc/kvm/book3s_xive.c | 15 ++++++++++++++
arch/powerpc/kvm/book3s_xive_native.c | 30 +++++++++++++++++++++++++++
3 files changed, 53 insertions(+)
@@ -946,6 +946,13 @@ int kvmppc_xive_set_mapped(struct kvm *kvm, unsigned long guest_irq,/* Turn the IPI hard off */xive_vm_esb_load(&state->ipi_data,XIVE_ESB_SET_PQ_01);+/*+*ResetESBguestmapping.NeededwhenESBpagesareexposed+*totheguestinXIVEnativemode+*/+if(xive->ops&&xive->ops->reset_mapped)+xive->ops->reset_mapped(kvm,guest_irq);+/* Grab info about irq */state->pt_number=hw_irq;state->pt_data=irq_data_get_irq_handler_data(host_data);
@@ -1031,6 +1038,14 @@ int kvmppc_xive_clr_mapped(struct kvm *kvm, unsigned long guest_irq,state->pt_number=0;state->pt_data=NULL;+/*+*ResetESBguestmapping.NeededwhenESBpagesareexposed+*totheguestinXIVEnativemode+*/+if(xive->ops&&xive->ops->reset_mapped){+xive->ops->reset_mapped(kvm,guest_irq);+}+/* Reconfigure the IPI */xive_native_configure_irq(state->ipi_number,xive_vp(xive,state->act_server),
@@ -240,6 +240,32 @@ static int kvmppc_xive_native_get_vc_base(struct kvmppc_xive *xive, u64 addr)return0;}+staticintkvmppc_xive_native_reset_mapped(structkvm*kvm,unsignedlongirq)+{+structkvmppc_xive*xive=kvm->arch.xive;+structmm_struct*mm=kvm->mm;+structvm_area_struct*vma=xive->vma;+unsignedlongaddress;++if(irq>=KVMPPC_XIVE_NR_IRQS)+return-EINVAL;++pr_debug("clearing esb pages for girq 0x%lx\n",irq);++down_read(&mm->mmap_sem);+/* TODO: can we clear the PTEs without keeping a VMA pointer ? */+if(vma){+address=vma->vm_start+irq*(2ull<<PAGE_SHIFT);+zap_vma_ptes(vma,address,2ull<<PAGE_SHIFT);+}+up_read(&mm->mmap_sem);+return0;+}++staticstructkvmppc_xive_opskvmppc_xive_native_ops={+.reset_mapped=kvmppc_xive_native_reset_mapped,+};+staticintxive_native_esb_fault(structvm_fault*vmf){structvm_area_struct*vma=vmf->vma;
@@ -292,6 +318,8 @@ static const struct vm_operations_struct xive_native_esb_vmops = {staticintxive_native_esb_mmap(structfile*file,structvm_area_struct*vma){+structkvmppc_xive*xive=vma->vm_file->private_data;+/* There are two ESB pages (trigger and EOI) per IRQ */if(vma_pages(vma)+vma->vm_pgoff>KVMPPC_XIVE_NR_IRQS*2)return-EINVAL;
@@ -299,6 +327,7 @@ static int xive_native_esb_mmap(struct file *file, struct vm_area_struct *vma)vma->vm_flags|=VM_IO|VM_PFNMAP;vma->vm_page_prot=pgprot_noncached(vma->vm_page_prot);vma->vm_ops=&xive_native_esb_vmops;+xive->vma=vma;/* TODO: get rid of the VMA pointer */return0;}
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-07 19:19:51
At a VCPU level, the state of the thread context interrupt management
registers needs to be collected. These registers are cached under the
'xive_saved_state.w01' field of the VCPU when the VPCU context is
pulled from the HW thread. An OPAL call retrieves the backup of the
IPB register in the NVT structure and merges it in the KVM state.
The structures of the interface between QEMU and KVM provisions some
extra room (two u64) for further extensions if more state needs to be
transferred back to QEMU.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/include/asm/kvm_ppc.h | 5 ++
arch/powerpc/include/uapi/asm/kvm.h | 2 +
arch/powerpc/kvm/book3s.c | 24 +++++++++
arch/powerpc/kvm/book3s_xive_native.c | 78 +++++++++++++++++++++++++++
4 files changed, 109 insertions(+)
@@ -424,6 +424,84 @@ static int xive_native_validate_queue_size(u32 qsize)}}+#define TM_IPB_SHIFT 40+#define TM_IPB_MASK (((u64) 0xFF) << TM_IPB_SHIFT)++intkvmppc_xive_native_get_vp(structkvm_vcpu*vcpu,unionkvmppc_one_reg*val)+{+structkvmppc_xive_vcpu*xc=vcpu->arch.xive_vcpu;+u64opal_state;+intrc;++if(!kvmppc_xive_enabled(vcpu))+return-EPERM;++if(!xc)+return-ENOENT;++/* Thread context registers. We only care about IPB and CPPR */+val->xive_timaval[0]=vcpu->arch.xive_saved_state.w01;++/*+*ReturntheOSCAMlinetoprintouttheVPidentifierin+*theQEMUmonitor.Thisisnotrestored.+*/+val->xive_timaval[1]=vcpu->arch.xive_cam_word;++/* Get the VP state from OPAL */+rc=xive_native_get_vp_state(xc->vp_id,&opal_state);+if(rc)+returnrc;++/*+*CapturethebackupofIPBregisterintheNVTstructureand+*mergeitinourKVMVPstate.+*+*TODO:P10support.+*/+val->xive_timaval[0]|=cpu_to_be64(opal_state&TM_IPB_MASK);++pr_devel("%s NSR=%02x CPPR=%02x IBP=%02x PIPR=%02x w01=%016llx w2=%08x opal=%016llx\n",+__func__,+vcpu->arch.xive_saved_state.nsr,+vcpu->arch.xive_saved_state.cppr,+vcpu->arch.xive_saved_state.ipb,+vcpu->arch.xive_saved_state.pipr,+vcpu->arch.xive_saved_state.w01,+(u32)vcpu->arch.xive_cam_word,opal_state);++return0;+}++intkvmppc_xive_native_set_vp(structkvm_vcpu*vcpu,unionkvmppc_one_reg*val)+{+structkvmppc_xive_vcpu*xc=vcpu->arch.xive_vcpu;+structkvmppc_xive*xive=vcpu->kvm->arch.xive;++pr_devel("%s w01=%016llx vp=%016llx\n",__func__,+val->xive_timaval[0],val->xive_timaval[1]);++if(!kvmppc_xive_enabled(vcpu))+return-EPERM;++if(!xc||!xive)+return-ENOENT;++/* We can't update the state of a "pushed" VCPU */+if(WARN_ON(vcpu->arch.xive_pushed))+return-EIO;++/* Thread context registers. only restore IPB and CPPR ? */+vcpu->arch.xive_saved_state.w01=val->xive_timaval[0];++/*+*ThereisnoneedtorestoretheXIVEinternalstate(IPB+*storedintheNVT)astheIPBregisterwasmergedinKVMVP+*state.+*/+return0;+}+staticintkvmppc_xive_native_set_source(structkvmppc_xive*xive,longirq,u64addr){
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-07 19:24:56
This will be used to destroy the KVM XICS or XIVE device when the
sPAPR machine is reseted. When the VM boots, the CAS negotiation
process will determine which interrupt mode to use and the appropriate
KVM device will then be created.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
include/linux/kvm_host.h | 2 ++
include/uapi/linux/kvm.h | 2 ++
arch/powerpc/kvm/book3s_xive.c | 38 +++++++++++++++++++++++++-
arch/powerpc/kvm/book3s_xive_native.c | 24 +++++++++++++++++
virt/kvm/kvm_main.c | 39 +++++++++++++++++++++++++++
5 files changed, 104 insertions(+), 1 deletion(-)
@@ -1109,11 +1109,19 @@ void kvmppc_xive_disable_vcpu_interrupts(struct kvm_vcpu *vcpu)voidkvmppc_xive_cleanup_vcpu(structkvm_vcpu*vcpu){structkvmppc_xive_vcpu*xc=vcpu->arch.xive_vcpu;-structkvmppc_xive*xive=xc->xive;+structkvmppc_xive*xive;inti;+if(!kvmppc_xics_enabled(vcpu))+return;++if(!xc)+return;+pr_devel("cleanup_vcpu(cpu=%d)\n",xc->server_num);+xive=xc->xive;+/* Ensure no interrupt is still routed to that VP */xc->valid=false;kvmppc_xive_disable_vcpu_interrupts(vcpu);
@@ -1150,6 +1158,10 @@ void kvmppc_xive_cleanup_vcpu(struct kvm_vcpu *vcpu)}/* Free the VP */kfree(xc);++/* Cleanup the vcpu */+vcpu->arch.irq_type=KVMPPC_IRQ_DEFAULT;+vcpu->arch.xive_vcpu=NULL;}intkvmppc_xive_connect_vcpu(structkvm_device*dev,
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-07 19:27:43
The support for XIVE native exploitation mode in Linux/KVM needs a
couple more OPAL calls to configure the sPAPR guest and to get/set the
state of the XIVE internal structures.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/include/asm/opal-api.h | 11 ++-
arch/powerpc/include/asm/opal.h | 7 ++
arch/powerpc/include/asm/xive.h | 14 +++
arch/powerpc/sysdev/xive/native.c | 99 +++++++++++++++++++
.../powerpc/platforms/powernv/opal-wrappers.S | 3 +
5 files changed, 130 insertions(+), 4 deletions(-)
@@ -711,3 +717,96 @@ bool xive_native_has_single_escalation(void)returnxive_has_single_esc;}EXPORT_SYMBOL_GPL(xive_native_has_single_escalation);++intxive_native_get_queue_info(u32vp_id,u32prio,+u64*out_qpage,+u64*out_qsize,+u64*out_qeoi_page,+u32*out_escalate_irq,+u64*out_qflags)+{+__be64qpage;+__be64qsize;+__be64qeoi_page;+__be32escalate_irq;+__be64qflags;+s64rc;++rc=opal_xive_get_queue_info(vp_id,prio,&qpage,&qsize,+&qeoi_page,&escalate_irq,&qflags);+if(rc){+pr_err("OPAL failed to get queue info for VCPU %d/%d : %lld\n",+vp_id,prio,rc);+return-EIO;+}++if(out_qpage)+*out_qpage=be64_to_cpu(qpage);+if(out_qsize)+*out_qsize=be32_to_cpu(qsize);+if(out_qeoi_page)+*out_qeoi_page=be64_to_cpu(qeoi_page);+if(out_escalate_irq)+*out_escalate_irq=be32_to_cpu(escalate_irq);+if(out_qflags)+*out_qflags=be64_to_cpu(qflags);++return0;+}+EXPORT_SYMBOL_GPL(xive_native_get_queue_info);++intxive_native_get_queue_state(u32vp_id,u32prio,u32*qtoggle,u32*qindex)+{+__be32opal_qtoggle;+__be32opal_qindex;+s64rc;++rc=opal_xive_get_queue_state(vp_id,prio,&opal_qtoggle,+&opal_qindex);+if(rc){+pr_err("OPAL failed to get queue state for VCPU %d/%d : %lld\n",+vp_id,prio,rc);+return-EIO;+}++if(qtoggle)+*qtoggle=be32_to_cpu(opal_qtoggle);+if(qindex)+*qindex=be32_to_cpu(opal_qindex);++return0;+}+EXPORT_SYMBOL_GPL(xive_native_get_queue_state);++intxive_native_set_queue_state(u32vp_id,u32prio,u32qtoggle,u32qindex)+{+s64rc;++rc=opal_xive_set_queue_state(vp_id,prio,qtoggle,qindex);+if(rc){+pr_err("OPAL failed to set queue state for VCPU %d/%d : %lld\n",+vp_id,prio,rc);+return-EIO;+}++return0;+}+EXPORT_SYMBOL_GPL(xive_native_set_queue_state);++intxive_native_get_vp_state(u32vp_id,u64*out_state)+{+__be64state;+s64rc;++rc=opal_xive_get_vp_state(vp_id,&state);+if(rc){+pr_err("OPAL failed to get vp state for VCPU %d : %lld\n",+vp_id,rc);+return-EIO;+}++if(out_state)+*out_state=be64_to_cpu(state);+return0;+}+EXPORT_SYMBOL_GPL(xive_native_get_vp_state);
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-07 19:29:10
This will let the guest create a memory mapping to expose the XIVE
MMIO region (TIMA) used for interrupt management at the CPU level.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/include/asm/xive.h | 1 +
arch/powerpc/include/uapi/asm/kvm.h | 1 +
arch/powerpc/kvm/book3s_xive_native.c | 57 +++++++++++++++++++++++++++
arch/powerpc/sysdev/xive/native.c | 11 ++++++
4 files changed, 70 insertions(+)
@@ -573,6 +576,14 @@ bool __init xive_native_init(void)for_each_possible_cpu(cpu)kvmppc_set_xive_tima(cpu,r.start,tima);+/* Resource 2 is OS window */+if(of_address_to_resource(np,2,&r)){+pr_err("Failed to get thread mgmnt area resource\n");+returnfalse;+}++xive_tima_os=r.start;+/* Grab size of provisionning pages */xive_parse_provisioning(np);
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-07 19:30:36
Interrupt sources are simply created at the OPAL level and then
MASKED. KVM only needs to know about their type: LSI or MSI.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/include/uapi/asm/kvm.h | 5 +
arch/powerpc/kvm/book3s_xive_native.c | 98 +++++++++++++++++++
.../powerpc/kvm/book3s_xive_native_template.c | 27 +++++
3 files changed, 130 insertions(+)
create mode 100644 arch/powerpc/kvm/book3s_xive_native_template.c
@@ -305,6 +323,78 @@ static int kvmppc_xive_native_get_tima_fd(struct kvmppc_xive *xive, u64 addr)returnput_user(ret,ubufp);}+staticintkvmppc_xive_native_set_source(structkvmppc_xive*xive,longirq,+u64addr)+{+structkvmppc_xive_src_block*sb;+structkvmppc_xive_irq_state*state;+u64__user*ubufp=(u64__user*)addr;+u64val;+u16idx;++pr_devel("%s irq=0x%lx\n",__func__,irq);++if(irq<KVMPPC_XIVE_FIRST_IRQ||irq>=KVMPPC_XIVE_NR_IRQS)+return-ENOENT;++sb=kvmppc_xive_find_source(xive,irq,&idx);+if(!sb){+pr_debug("No source, creating source block...\n");+sb=kvmppc_xive_create_src_block(xive,irq);+if(!sb){+pr_err("Failed to create block...\n");+return-ENOMEM;+}+}+state=&sb->irq_state[idx];++if(get_user(val,ubufp)){+pr_err("fault getting user info !\n");+return-EFAULT;+}++/*+*Ifthesourcedoesn'talreadyhaveanIPI,allocate+*oneandgetthecorrespondingdata+*/+if(!state->ipi_number){+state->ipi_number=xive_native_alloc_irq();+if(state->ipi_number==0){+pr_err("Failed to allocate IRQ !\n");+return-ENOMEM;+}+xive_native_populate_irq_data(state->ipi_number,+&state->ipi_data);+pr_debug("%s allocated hw_irq=0x%x for irq=0x%lx\n",__func__,+state->ipi_number,irq);+}++arch_spin_lock(&sb->lock);++/* Restore LSI state */+if(val&KVM_XIVE_LEVEL_SENSITIVE){+state->lsi=true;+if(val&KVM_XIVE_LEVEL_ASSERTED)+state->asserted=true;+pr_devel(" LSI ! Asserted=%d\n",state->asserted);+}++/* Mask IRQ to start with */+state->act_server=0;+state->act_priority=MASKED;+xive_vm_esb_load(&state->ipi_data,XIVE_ESB_SET_PQ_01);+xive_native_configure_irq(state->ipi_number,0,MASKED,0);++/* Increment the number of valid sources and mark this one valid */+if(!state->valid)+xive->src_count++;+state->valid=true;++arch_spin_unlock(&sb->lock);++return0;+}+staticintkvmppc_xive_native_set_attr(structkvm_device*dev,structkvm_device_attr*attr){
@@ -317,6 +407,9 @@ static int kvmppc_xive_native_set_attr(struct kvm_device *dev,returnkvmppc_xive_native_set_vc_base(xive,attr->addr);}break;+caseKVM_DEV_XIVE_GRP_SOURCES:+returnkvmppc_xive_native_set_source(xive,attr->attr,+attr->addr);}return-ENXIO;}
@@ -353,6 +446,11 @@ static int kvmppc_xive_native_has_attr(struct kvm_device *dev,return0;}break;+caseKVM_DEV_XIVE_GRP_SOURCES:+if(attr->attr>=KVMPPC_XIVE_FIRST_IRQ&&+attr->attr<KVMPPC_XIVE_NR_IRQS)+return0;+break;}return-ENXIO;}
@@ -0,0 +1,27 @@+// SPDX-License-Identifier: GPL-2.0+/*+*Copyright(c)2017-2019,IBMCorporation.+*/++/* File to be included by other .c files */++#define XGLUE(a, b) a##b+#define GLUE(a, b) XGLUE(a, b)++/*+*TODO:introduceacommontemplatefilewiththeXIVEnativelayer+*andtheXICS-on-XIVEgluefortheutilityfunctions+*/+staticu8GLUE(X_PFX,esb_load)(structxive_irq_data*xd,u32offset)+{+u64val;++if(xd->flags&XIVE_IRQ_FLAG_SHIFT_BUG)+offset|=offset<<4;++val=__x_readq(__x_eoi_page(xd)+offset);+#ifdef __LITTLE_ENDIAN__+val>>=64-8;+#endif+return(u8)val;+}
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-07 20:03:58
The ESB MMIO region controls the interrupt sources of the guest. QEMU
will query an fd (GET_ESB_FD ioctl) and map this region at a specific
address for the guest to use. The guest will obtain this information
using the H_INT_GET_SOURCE_INFO hcall. To inform KVM of the address
setting used by QEMU, add a VC_BASE control to the KVM XIVE device
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/include/uapi/asm/kvm.h | 1 +
arch/powerpc/kvm/book3s_xive.h | 3 +++
arch/powerpc/kvm/book3s_xive_native.c | 39 +++++++++++++++++++++++++++
3 files changed, 43 insertions(+)
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-07 20:05:36
The Effective IRQ Source Number is the interrupt number pushed in the
event queue that the guest OS will use to dispatch events internally.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/kvm/book3s_xive.h | 3 +++
arch/powerpc/kvm/book3s_xive.c | 1 +
2 files changed, 4 insertions(+)
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-07 20:06:59
When migration of a VM is initiated, a first copy of the RAM is
transferred to the destination before the VM is stopped. At that time,
QEMU needs to perform a XIVE quiesce sequence to stop the flow of
event notifications and stabilize the EQs. The sources are masked and
the XIVE IC is synced with the KVM ioctl KVM_DEV_XIVE_GRP_SYNC.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/include/uapi/asm/kvm.h | 1 +
arch/powerpc/kvm/book3s_xive_native.c | 32 +++++++++++++++++++++++++++
2 files changed, 33 insertions(+)
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-07 20:08:16
These are used to capture the XIVE END table of the KVM device. It
relies on an OPAL call to retrieve from the XIVE IC the EQ toggle bit
and index which are updated by the HW when events are enqueued in the
guest RAM.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/include/uapi/asm/kvm.h | 21 ++++
arch/powerpc/kvm/book3s_xive_native.c | 166 ++++++++++++++++++++++++++
2 files changed, 187 insertions(+)
From: David Gibson <hidden> Date: 2019-01-09 03:47:14
On Mon, Jan 07, 2019 at 07:43:13PM +0100, Cédric Le Goater wrote:
These flags are shared between Linux/KVM implementing the hypervisor
calls for the XIVE native exploitation mode and the driver for the
sPAPR guests.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
@@ -432,7 +420,7 @@ static int xive_spapr_configure_queue(u32 target, struct xive_q *q, u8 prio,q->eoi_phys=esn_page;/* Default is to always notify */-flags=XIVE_EQ_ALWAYS_NOTIFY;+flags=XIVE_SPAPR_EQ_ALWAYS_NOTIFY;/* Configure and enable the queue in HW */rc=plpar_int_set_queue_config(flags,target,prio,qpage_phys,order);
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
From: David Gibson <hidden> Date: 2019-01-09 04:57:23
On Mon, Jan 07, 2019 at 07:43:14PM +0100, Cédric Le Goater wrote:
The support for XIVE native exploitation mode in Linux/KVM needs a
couple more OPAL calls to configure the sPAPR guest and to get/set the
state of the XIVE internal structures.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
@@ -711,3 +717,96 @@ bool xive_native_has_single_escalation(void)returnxive_has_single_esc;}EXPORT_SYMBOL_GPL(xive_native_has_single_escalation);++intxive_native_get_queue_info(u32vp_id,u32prio,+u64*out_qpage,+u64*out_qsize,+u64*out_qeoi_page,+u32*out_escalate_irq,+u64*out_qflags)+{+__be64qpage;+__be64qsize;+__be64qeoi_page;+__be32escalate_irq;+__be64qflags;+s64rc;++rc=opal_xive_get_queue_info(vp_id,prio,&qpage,&qsize,+&qeoi_page,&escalate_irq,&qflags);+if(rc){+pr_err("OPAL failed to get queue info for VCPU %d/%d : %lld\n",+vp_id,prio,rc);+return-EIO;+}++if(out_qpage)+*out_qpage=be64_to_cpu(qpage);+if(out_qsize)+*out_qsize=be32_to_cpu(qsize);+if(out_qeoi_page)+*out_qeoi_page=be64_to_cpu(qeoi_page);+if(out_escalate_irq)+*out_escalate_irq=be32_to_cpu(escalate_irq);+if(out_qflags)+*out_qflags=be64_to_cpu(qflags);++return0;+}+EXPORT_SYMBOL_GPL(xive_native_get_queue_info);++intxive_native_get_queue_state(u32vp_id,u32prio,u32*qtoggle,u32*qindex)+{+__be32opal_qtoggle;+__be32opal_qindex;+s64rc;++rc=opal_xive_get_queue_state(vp_id,prio,&opal_qtoggle,+&opal_qindex);+if(rc){+pr_err("OPAL failed to get queue state for VCPU %d/%d : %lld\n",+vp_id,prio,rc);+return-EIO;+}++if(qtoggle)+*qtoggle=be32_to_cpu(opal_qtoggle);+if(qindex)+*qindex=be32_to_cpu(opal_qindex);++return0;+}+EXPORT_SYMBOL_GPL(xive_native_get_queue_state);++intxive_native_set_queue_state(u32vp_id,u32prio,u32qtoggle,u32qindex)+{+s64rc;++rc=opal_xive_set_queue_state(vp_id,prio,qtoggle,qindex);+if(rc){+pr_err("OPAL failed to set queue state for VCPU %d/%d : %lld\n",+vp_id,prio,rc);+return-EIO;+}++return0;+}+EXPORT_SYMBOL_GPL(xive_native_set_queue_state);++intxive_native_get_vp_state(u32vp_id,u64*out_state)+{+__be64state;+s64rc;++rc=opal_xive_get_vp_state(vp_id,&state);+if(rc){+pr_err("OPAL failed to get vp state for VCPU %d : %lld\n",+vp_id,rc);+return-EIO;+}++if(out_state)+*out_state=be64_to_cpu(state);+return0;+}+EXPORT_SYMBOL_GPL(xive_native_get_vp_state);
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
From: David Gibson <hidden> Date: 2019-01-09 04:58:50
On Mon, Jan 07, 2019 at 07:43:15PM +0100, Cédric Le Goater wrote:
We will have different KVM devices for interrupts, one for the
XICS-over-XIVE mode and one for the XIVE native exploitation
mode. Let's add some checks to make sure we are not mixing the
interfaces in KVM.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
@@ -835,6 +838,9 @@ int kvmppc_xive_set_icp(struct kvm_vcpu *vcpu, u64 icpval)u8cppr,mfrr;u32xisr;+if(!kvmppc_xics_enabled(vcpu))+return-EPERM;+if(!xc||!xive)return-ENOENT;
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2019-01-09 13:10:49
Cédric Le Goater [off-list ref] writes:
quoted hunk
These flags are shared between Linux/KVM implementing the hypervisor
calls for the XIVE native exploitation mode and the driver for the
sPAPR guests.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/include/asm/xive.h | 23 +++++++++++++++++++++++
arch/powerpc/sysdev/xive/spapr.c | 28 ++++++++--------------------
2 files changed, 31 insertions(+), 20 deletions(-)
I have an (irrational) hatred of PPC_BIT, because it obfuscates what's
going on and makes PPC seem weirder than it needs to be. It could at
least be called IBM_BIT().
I know it helps people compare the code vs the documentation, but
basically no one has the documentation, and everyone has the code.
Anyway it's not a show stopper, just a pet-peeve of mine :)
cheers
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-09 13:45:02
On 1/9/19 2:08 PM, Michael Ellerman wrote:
Cédric Le Goater [off-list ref] writes:
quoted
These flags are shared between Linux/KVM implementing the hypervisor
calls for the XIVE native exploitation mode and the driver for the
sPAPR guests.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/include/asm/xive.h | 23 +++++++++++++++++++++++
arch/powerpc/sysdev/xive/spapr.c | 28 ++++++++--------------------
2 files changed, 31 insertions(+), 20 deletions(-)
I have an (irrational) hatred of PPC_BIT, because it obfuscates what's
going on and makes PPC seem weirder than it needs to be. It could at
least be called IBM_BIT().
I know it helps people compare the code vs the documentation, but
basically no one has the documentation, and everyone has the code.
Anyway it's not a show stopper, just a pet-peeve of mine :)
Only the define matters, I can change that back to the non-PPC_BIT
version in v2. Not a problem.
Cheers,
C.
From: David Gibson <hidden> Date: 2019-01-11 04:51:37
On Mon, Jan 07, 2019 at 07:43:16PM +0100, Cédric Le Goater wrote:
The KVM device for the XIVE native exploitation mode will reuse the
structures of the XICS-over-XIVE glue implementation. Some code will
also be shared : source block creation and destruction, target
selection and escalation attachment.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
From: Paul Mackerras <hidden> Date: 2019-01-22 04:48:46
On Mon, Jan 07, 2019 at 07:43:12PM +0100, Cédric Le Goater wrote:
Hello,
On the POWER9 processor, the XIVE interrupt controller can control
interrupt sources using MMIO to trigger events, to EOI or to turn off
the sources. Priority management and interrupt acknowledgment is also
controlled by MMIO in the CPU presenter subengine.
PowerNV/baremetal Linux runs natively under XIVE but sPAPR guests need
special support from the hypervisor to do the same. This is called the
XIVE native exploitation mode and today, it can be activated under the
PowerPC Hypervisor, pHyp. However, Linux/KVM lacks XIVE native support
and still offers the old interrupt mode interface using a
XICS-over-XIVE glue which implements the XICS hcalls.
The following series is proposal to add the same support under KVM.
A new KVM device is introduced for the XIVE native exploitation
mode. It reuses most of the XICS-over-XIVE glue implementation
structures which are internal to KVM but has a completely different
interface. A set of Hypervisor calls configures the sources and the
event queues and from there, all control is done by the guest through
MMIOs.
These MMIO regions (ESB and TIMA) are exposed to guests in QEMU,
similarly to VFIO, and the associated VMAs are populated dynamically
with the appropriate pages using a fault handler. This is implemented
with a couple of KVM device ioctls.
On a POWER9 sPAPR machine, the Client Architecture Support (CAS)
negotiation process determines whether the guest operates with a
interrupt controller using the XICS legacy model, as found on POWER8,
or in XIVE exploitation mode. Which means that the KVM interrupt
device should be created at runtime, after the machine as started.
This requires extra KVM support to create/destroy KVM devices. The
last patches are an attempt to solve that problem.
Migration has its own specific needs. The patchset provides the
necessary routines to quiesce XIVE, to capture and restore the state
of the different structures used by KVM, OPAL and HW. Extra OPAL
support is required for these.
Thanks for the patchset. It mostly looks good, but there are some
more things we need to consider, and I think a v2 will be needed.
One general comment I have is that there are a lot of acronyms in this
code and you mostly seem to assume that people will know what they all
mean. It would make the code more readable if you provide the
expansion of the acronym on first use in a comment or whatever. For
example, one of the patches in this series talks about the "EAS"
without ever expanding it in any comment or in the patch description,
and I have forgotten just at the moment what EAS stands for (I just
know that understanding the XIVE is not eas-y. :)
Another general comment is that you seem to have written all this
code assuming we are using HV KVM in a host running bare-metal.
However, we could be using PR KVM (either in a bare-metal host or in a
guest), or we could be doing nested HV KVM where we are using the
kvm_hv module inside a KVM guest and using special hypercalls for
controlling our guests.
It would be perfectly acceptable for now to say that we don't yet
support XIVE exploitation in those scenarios, as long as we then make
sure that the new KVM capability reports false in those scenarios, and
any attempt to use the XIVE exploitation interfaces fails cleanly.
I don't see that either of those is true in the patch set as it
stands, so that is one area that needs to be fixed.
A third general comment is that the new KVM interfaces you have added
need to be documented in the files under Documentation/virtual/kvm.
Paul.
From: Paul Mackerras <hidden> Date: 2019-01-22 05:45:30
On Mon, Jan 07, 2019 at 07:43:17PM +0100, Cédric Le Goater wrote:
This is the basic framework for the new KVM device supporting the XIVE
native exploitation mode. The user interface exposes a new capability
and a new KVM device to be used by QEMU.
[snip]
quoted hunk
@@ -1039,7 +1039,10 @@ static int kvmppc_book3s_init(void) #ifdef CONFIG_KVM_XIVE if (xive_enabled()) { kvmppc_xive_init_module();+ kvmppc_xive_native_init_module(); kvm_register_device_ops(&kvm_xive_ops, KVM_DEV_TYPE_XICS);+ kvm_register_device_ops(&kvm_xive_native_ops,+ KVM_DEV_TYPE_XIVE);
I think we want tighter conditions on initializing the xive_native
stuff and creating the xive device class. We could have
xive_enabled() returning true in a guest, and this code will get
called both by PR KVM and HV KVM (and HV KVM no longer implies that we
are running bare metal).
quoted hunk
@@ -1050,8 +1053,10 @@ static int kvmppc_book3s_init(void) static void kvmppc_book3s_exit(void) { #ifdef CONFIG_KVM_XICS- if (xive_enabled())+ if (xive_enabled()) { kvmppc_xive_exit_module();+ kvmppc_xive_native_exit_module();
From: Paul Mackerras <hidden> Date: 2019-01-22 05:47:03
On Mon, Jan 07, 2019 at 08:10:05PM +0100, Cédric Le Goater wrote:
Clear the ESB pages from the VMA of the IRQ being pass through to the
guest and let the fault handler repopulate the VMA when the ESB pages
are accessed for an EOI or for a trigger.
Why do we want to do this?
I don't see any possible advantage to removing the PTEs from the
userspace mapping. You'll need to explain further.
Paul.
From: Paul Mackerras <hidden> Date: 2019-01-22 05:48:29
On Mon, Jan 07, 2019 at 07:43:15PM +0100, Cédric Le Goater wrote:
quoted hunk
We will have different KVM devices for interrupts, one for the
XICS-over-XIVE mode and one for the XIVE native exploitation
mode. Let's add some checks to make sure we are not mixing the
interfaces in KVM.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/kvm/book3s_xive.c | 6 ++++++
1 file changed, 6 insertions(+)
@@ -835,6 +838,9 @@ int kvmppc_xive_set_icp(struct kvm_vcpu *vcpu, u64 icpval)u8cppr,mfrr;u32xisr;+if(!kvmppc_xics_enabled(vcpu))+return-EPERM;+if(!xc||!xive)return-ENOENT;
I can't see how these new checks could ever trigger in the code as it
stands. Is there a way at present? Do following patches ever add a
path where the new checks could trigger, or is this just an excess of
caution? (Your patch description should ideally have answered these
questions for me.)
Paul.
From: Paul Mackerras <hidden> Date: 2019-01-22 05:50:00
On Mon, Jan 07, 2019 at 07:43:18PM +0100, Cédric Le Goater wrote:
quoted hunk
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@kaod.org>
---
arch/powerpc/include/uapi/asm/kvm.h | 4 ++
arch/powerpc/kvm/book3s_xive_native.c | 97 +++++++++++++++++++++++++++
2 files changed, 101 insertions(+)
@@ -153,6 +153,85 @@ int kvmppc_xive_native_connect_vcpu(struct kvm_device *dev,returnrc;}+staticintxive_native_esb_fault(structvm_fault*vmf)+{+structvm_area_struct*vma=vmf->vma;+structkvmppc_xive*xive=vma->vm_file->private_data;+structkvmppc_xive_src_block*sb;+structkvmppc_xive_irq_state*state;+structxive_irq_data*xd;+u32hw_num;+u16src;+u64page;+unsignedlongirq;++/*+*Linux/KVMusesatwopagesESBsetting,onefortriggerand+*oneforEOI+*/+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)?
quoted hunk
+ 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.
From: Paul Mackerras <hidden> Date: 2019-01-22 05:51:33
On Mon, Jan 07, 2019 at 08:10:06PM +0100, Cédric Le Goater wrote:
This will be used to destroy the KVM XICS or XIVE device when the
sPAPR machine is reseted. When the VM boots, the CAS negotiation
process will determine which interrupt mode to use and the appropriate
KVM device will then be created.
What would be the consequence if we didn't destroy the device?
The reason I ask is that we will have to be much more careful about
memory allocation lifetimes with this patch. Having KVM devices last
until the KVM instance is destroyed means that we generally avoid
use-after-free bugs. With this patch we will have to do a careful
analysis of the lifetime of the xive structures vs. possible accesses
on other threads to prove there are no use-after-free bugs.
For example, it is not sufficient to set any pointers in struct kvm or
struct kvm_vcpu that point into xive structures to NULL before freeing
the structures. There could be code on another CPU that has read the
pointer value before you set it to NULL and then goes and accesses it
after you have freed it. You need to prove that can't happen,
possibly using some sort of explicit synchronization that ensures that
no other CPU could still be accessing the structure at the time when
you free it. RCU can help with this, but in general means you need
RCU synchronization primitives (rcu_read_lock() etc.) at all the
places where you use the pointer, which I don't think you currently
have.
If there is a good fundamental reason why this can't happen, even
though you don't have explicit synchronization, then at a minimum you
need to explain that in the patch description, and ideally also in
code comments.
Paul.
From: Paul Mackerras <hidden> Date: 2019-01-22 05:53:30
On Mon, Jan 07, 2019 at 07:43:23PM +0100, Cédric Le Goater wrote:
The XIVE native exploitation mode specs define a set of Hypervisor
calls to configure the sources and the event queues :
- H_INT_GET_SOURCE_INFO
used to obtain the address of the MMIO page of the Event State
Buffer (PQ bits) entry associated with the source.
- H_INT_SET_SOURCE_CONFIG
assigns a source to a "target".
- H_INT_GET_SOURCE_CONFIG
determines which "target" and "priority" is assigned to a source
- H_INT_GET_QUEUE_INFO
returns the address of the notification management page associated
with the specified "target" and "priority".
- H_INT_SET_QUEUE_CONFIG
sets or resets the event queue for a given "target" and "priority".
It is also used to set the notification configuration associated
with the queue, only unconditional notification is supported for
the moment. Reset is performed with a queue size of 0 and queueing
is disabled in that case.
- H_INT_GET_QUEUE_CONFIG
returns the queue settings for a given "target" and "priority".
- H_INT_RESET
resets all of the guest's internal interrupt structures to their
initial state, losing all configuration set via the hcalls
H_INT_SET_SOURCE_CONFIG and H_INT_SET_QUEUE_CONFIG.
- H_INT_SYNC
issue a synchronisation on a source to make sure all notifications
have reached their queue.
Which ones of these could be implemented in QEMU? Are there any that
can't possibly be implemented in QEMU because they need to do things
that require calling internal interfaces that userspace doesn't have
access to?
How often do we expect each of these hypercalls to be called?
[snip]
quoted hunk
@@ -682,6 +685,46 @@ int kvmppc_rm_h_cppr(struct kvm_vcpu *vcpu, unsigned long cppr); int kvmppc_rm_h_eoi(struct kvm_vcpu *vcpu, unsigned long xirr); void kvmppc_guest_entry_inject_int(struct kvm_vcpu *vcpu);+int kvmppc_rm_h_int_get_source_info(struct kvm_vcpu *vcpu,+ unsigned long flag,+ unsigned long lisn);+int kvmppc_rm_h_int_set_source_config(struct kvm_vcpu *vcpu,+ unsigned long flag,+ unsigned long lisn,+ unsigned long target,+ unsigned long priority,+ unsigned long eisn);+int kvmppc_rm_h_int_get_source_config(struct kvm_vcpu *vcpu,+ unsigned long flag,+ unsigned long lisn);+int kvmppc_rm_h_int_get_queue_info(struct kvm_vcpu *vcpu,+ unsigned long flag,+ unsigned long target,+ unsigned long priority);+int kvmppc_rm_h_int_set_queue_config(struct kvm_vcpu *vcpu,+ unsigned long flag,+ unsigned long target,+ unsigned long priority,+ unsigned long qpage,+ unsigned long qsize);+int kvmppc_rm_h_int_get_queue_config(struct kvm_vcpu *vcpu,+ unsigned long flag,+ unsigned long target,+ unsigned long priority);+int kvmppc_rm_h_int_set_os_reporting_line(struct kvm_vcpu *vcpu,+ unsigned long flag,+ unsigned long reportingline);+int kvmppc_rm_h_int_get_os_reporting_line(struct kvm_vcpu *vcpu,+ unsigned long flag,+ unsigned long target,+ unsigned long reportingline);+int kvmppc_rm_h_int_esb(struct kvm_vcpu *vcpu, unsigned long flag,+ unsigned long lisn, unsigned long offset,+ unsigned long data);+int kvmppc_rm_h_int_sync(struct kvm_vcpu *vcpu, unsigned long flag,+ unsigned long lisn);+int kvmppc_rm_h_int_reset(struct kvm_vcpu *vcpu, unsigned long flag);
Why do we need to provide real-mode versions of these hypercall
handlers? I thought these hypercalls would only get called
infrequently, and in any case certainly much less frequently than once
per interrupt delivered. If they are infrequent, then let's leave out
the real-mode version and just handle them in book3s_hv.c.
From: Paul Mackerras <hidden> Date: 2019-01-22 05:55:08
On Mon, Jan 07, 2019 at 07:43:20PM +0100, Cédric Le Goater wrote:
The ESB MMIO region controls the interrupt sources of the guest. QEMU
will query an fd (GET_ESB_FD ioctl) and map this region at a specific
address for the guest to use. The guest will obtain this information
using the H_INT_GET_SOURCE_INFO hcall. To inform KVM of the address
setting used by QEMU, add a VC_BASE control to the KVM XIVE device
This needs a little more explanation. I *think* the only way this
gets used is that it gets returned to the guest by the new
hypercalls. If that is indeed the case it would be useful to mention
that in the patch description, because otherwise taking a value that
userspace provides and which looks like it is an address, and not
doing any validation on it, looks a bit scary.
Paul.
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2019-01-23 06:47:31
On Tue, 2019-01-22 at 16:26 +1100, Paul Mackerras wrote:
On Mon, Jan 07, 2019 at 08:10:05PM +0100, Cédric Le Goater wrote:
quoted
Clear the ESB pages from the VMA of the IRQ being pass through to the
guest and let the fault handler repopulate the VMA when the ESB pages
are accessed for an EOI or for a trigger.
Why do we want to do this?
I don't see any possible advantage to removing the PTEs from the
userspace mapping. You'll need to explain further.
Afaik bcs we change the mapping to point to the real HW irq ESB page
instead of the "IPI" that was there at VM init time.
Cedric, is that correct ?
Cheers,
Ben.
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-23 08:51:38
On 1/23/19 7:44 AM, Benjamin Herrenschmidt wrote:
On Tue, 2019-01-22 at 16:23 +1100, Paul Mackerras wrote:
quoted
Which ones of these could be implemented in QEMU? Are there any that
can't possibly be implemented in QEMU because they need to do things
that require calling internal interfaces that userspace doesn't have
access to?
Implementing them in qemu doesn't make a lot of sense. Qemu doesn't
have access to most of the XIVE HW state. There's a XIVE model for full
emulation but when using the real thing, almost none of it is visible.
A lot of those hcalls effectively turn into OPAL calls.
quoted
How often do we expect each of these hypercalls to be called?
It depends, I can't tell for AIX. For Linux, not often with one
exception: H_INT_ESB which will be used whenever you EOI an emulated
LSI.
yes. This one is only doing loads and stores.
.../...
quoted
Why do we need to provide real-mode versions of these hypercall
handlers? I thought these hypercalls would only get called
infrequently, and in any case certainly much less frequently than once
per interrupt delivered. If they are infrequent, then let's leave out
the real-mode version and just handle them in book3s_hv.c.
Agreed with the exception maybe of H_INT_ESB
ok.
Some of these hcalls are really simple and only getting local info from
the host (h_int_get_*). I thought handling the hcall ASAP was a preferred
practice, even if the hcall is not called frequently. Isn't it ?
From: Paul Mackerras <hidden> Date: 2019-01-23 10:32:05
On Wed, Jan 23, 2019 at 05:45:24PM +1100, Benjamin Herrenschmidt wrote:
On Tue, 2019-01-22 at 16:26 +1100, Paul Mackerras wrote:
quoted
On Mon, Jan 07, 2019 at 08:10:05PM +0100, Cédric Le Goater wrote:
quoted
Clear the ESB pages from the VMA of the IRQ being pass through to the
guest and let the fault handler repopulate the VMA when the ESB pages
are accessed for an EOI or for a trigger.
Why do we want to do this?
I don't see any possible advantage to removing the PTEs from the
userspace mapping. You'll need to explain further.
Afaik bcs we change the mapping to point to the real HW irq ESB page
instead of the "IPI" that was there at VM init time.
So that makes it sound like there is a whole lot going on that hasn't
even been hinted at in the patch descriptions... It sounds like we
need a good description of how all this works and fits together
somewhere under Documentation/.
In any case we need much more informative patch descriptions. I
realize that it's all currently in Cedric's head, but I bet that in
two or three years' time when we come to try to debug something, it
won't be in anyone's head...
Paul.
From: Paul Mackerras <hidden> Date: 2019-01-23 10:33:37
On Wed, Jan 23, 2019 at 09:48:31AM +0100, Cédric Le Goater wrote:
On 1/23/19 7:44 AM, Benjamin Herrenschmidt wrote:
quoted
On Tue, 2019-01-22 at 16:23 +1100, Paul Mackerras wrote:
quoted
Why do we need to provide real-mode versions of these hypercall
handlers? I thought these hypercalls would only get called
infrequently, and in any case certainly much less frequently than once
per interrupt delivered. If they are infrequent, then let's leave out
the real-mode version and just handle them in book3s_hv.c.
Agreed with the exception maybe of H_INT_ESB
ok.
Some of these hcalls are really simple and only getting local info from
the host (h_int_get_*). I thought handling the hcall ASAP was a preferred
practice, even if the hcall is not called frequently. Isn't it ?
If we are going to handle a given hcall in the kernel at all, then we
have to have a virtual mode handler. If we have a real-mode handler
as well then we in general incur a certain amount of code duplication
with consequent maintenance costs and possibility of bugs. So we
generally only have real-mode handlers for the hcalls where it is
critical to minimize the latency. From what Ben is saying that would
only be H_INT_ESB, and maybe not even that.
If H_INT_ESB is only used for LSIs, then is a guest going to be using
it at all? My understanding was that with XIVE, only a small number
of interrupts that are to do with system management functions are
LSIs; all of the interrupts relating to PCI-e devices are MSIs. So do
we actually have a real high-frequency use case for LSIs in a guest?
For now I would prefer that you remove all the real-mode hcall
handlers. We can add them later if we get performance data showing
that they are needed.
Regarding whether or not to have a given hcall handler in the kernel
at all - if there is for example an hcall which is just called once
on guest startup, and its function is just to provide information to
the guest, and QEMU has that information, then why not have that hcall
implemented by QEMU? Are any of the hcalls like that?
For example, if H_INT_GET_SOURCE_INFO was implemented in QEMU, could
we then remove the VC_BASE thing from the xive device?
Paul.
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-23 11:09:23
On 1/23/19 11:26 AM, Paul Mackerras wrote:
On Wed, Jan 23, 2019 at 09:48:31AM +0100, Cédric Le Goater wrote:
quoted
On 1/23/19 7:44 AM, Benjamin Herrenschmidt wrote:
quoted
On Tue, 2019-01-22 at 16:23 +1100, Paul Mackerras wrote:
quoted
Why do we need to provide real-mode versions of these hypercall
handlers? I thought these hypercalls would only get called
infrequently, and in any case certainly much less frequently than once
per interrupt delivered. If they are infrequent, then let's leave out
the real-mode version and just handle them in book3s_hv.c.
Agreed with the exception maybe of H_INT_ESB
ok.
Some of these hcalls are really simple and only getting local info from
the host (h_int_get_*). I thought handling the hcall ASAP was a preferred
practice, even if the hcall is not called frequently. Isn't it ?
If we are going to handle a given hcall in the kernel at all, then we
have to have a virtual mode handler. If we have a real-mode handler
as well then we in general incur a certain amount of code duplication
with consequent maintenance costs and possibility of bugs. So we
generally only have real-mode handlers for the hcalls where it is
critical to minimize the latency. From what Ben is saying that would
only be H_INT_ESB, and maybe not even that.
ok. and yes, even the H_INT_ESB is questionable as this is really a rare
configuration.
If H_INT_ESB is only used for LSIs, then is a guest going to be using
it at all? My understanding was that with XIVE, only a small number
of interrupts that are to do with system management functions are
LSIs; all of the interrupts relating to PCI-e devices are MSIs. So do
we actually have a real high-frequency use case for LSIs in a guest?
The guest should be using a rtl8139 or a e1000 NIC under QEMU/KVM, which
is not the common scenario.
For now I would prefer that you remove all the real-mode hcall
handlers. We can add them later if we get performance data showing
that they are needed.
ok. I will.
Regarding whether or not to have a given hcall handler in the kernel
at all - if there is for example an hcall which is just called once
on guest startup, and its function is just to provide information to
the guest, and QEMU has that information, then why not have that hcall
implemented by QEMU? Are any of the hcalls like that?
For example, if H_INT_GET_SOURCE_INFO was implemented in QEMU, could
we then remove the VC_BASE thing from the xive device?
Yes. H_INT_GET_SOURCE_INFO looks like a good candidate, all info should
be in QEMU and there are no OPAL calls, and we would get rid of the
VC_BASE kvm device ioctl at the same time.
Thanks,
C.
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-23 11:28:58
On 1/23/19 11:30 AM, Paul Mackerras wrote:
On Wed, Jan 23, 2019 at 05:45:24PM +1100, Benjamin Herrenschmidt wrote:
quoted
On Tue, 2019-01-22 at 16:26 +1100, Paul Mackerras wrote:
quoted
On Mon, Jan 07, 2019 at 08:10:05PM +0100, Cédric Le Goater wrote:
quoted
Clear the ESB pages from the VMA of the IRQ being pass through to the
guest and let the fault handler repopulate the VMA when the ESB pages
are accessed for an EOI or for a trigger.
Why do we want to do this?
I don't see any possible advantage to removing the PTEs from the
userspace mapping. You'll need to explain further.
Afaik bcs we change the mapping to point to the real HW irq ESB page
instead of the "IPI" that was there at VM init time.
yes exactly. You need to clean up the pages each time.
So that makes it sound like there is a whole lot going on that hasn't
even been hinted at in the patch descriptions... It sounds like we
need a good description of how all this works and fits together
somewhere under Documentation/.
OK. I have started doing so for the models merged in QEMU but not yet
for KVM. I will work on it.
In any case we need much more informative patch descriptions. I
realize that it's all currently in Cedric's head, but I bet that in
two or three years' time when we come to try to debug something, it
won't be in anyone's head...
I agree.
So, storing the ESB VMA under the KVM device is not shocking anyone ?
Thanks,
C.
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-23 16:26:11
On 1/22/19 5:56 AM, Paul Mackerras wrote:
On Mon, Jan 07, 2019 at 07:43:15PM +0100, Cédric Le Goater wrote:
quoted
We will have different KVM devices for interrupts, one for the
XICS-over-XIVE mode and one for the XIVE native exploitation
mode. Let's add some checks to make sure we are not mixing the
interfaces in KVM.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/kvm/book3s_xive.c | 6 ++++++
1 file changed, 6 insertions(+)
@@ -835,6 +838,9 @@ int kvmppc_xive_set_icp(struct kvm_vcpu *vcpu, u64 icpval)u8cppr,mfrr;u32xisr;+if(!kvmppc_xics_enabled(vcpu))+return-EPERM;+if(!xc||!xive)return-ENOENT;
I can't see how these new checks could ever trigger in the code as it
stands. Is there a way at present?
It would require some custom QEMU doing silly things : create the XICS
KVM device, and then call kvm_get_one_reg(KVM_REG_PPC_ICP_STATE) or
kvm_set_one_reg(icp->cs, KVM_REG_PPC_ICP_STATE) without connecting the
vCPU to its presenter.
Today, you get a ENOENT.
Do following patches ever add a path where the new checks could trigger,
or is this just an excess of caution?
With the following patches, QEMU could to do something even more silly,
which is to mix the interrupt mode interfaces : create a KVM XICS device
and call KVM CPU ioctls of the KVM XIVE device, or the opposite.
(Your patch description should ideally have answered these questions > for me.)
Yes. I also think that I introduced this patch to early in the series.
It make more sense when the XICS and the XIVE KVM devices are available.
Thanks,
C.
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-23 16:30:26
On 1/22/19 6:05 AM, Paul Mackerras wrote:
On Mon, Jan 07, 2019 at 07:43:17PM +0100, Cédric Le Goater wrote:
quoted
This is the basic framework for the new KVM device supporting the XIVE
native exploitation mode. The user interface exposes a new capability
and a new KVM device to be used by QEMU.
[snip]
quoted
@@ -1039,7 +1039,10 @@ static int kvmppc_book3s_init(void) #ifdef CONFIG_KVM_XIVE if (xive_enabled()) { kvmppc_xive_init_module();+ kvmppc_xive_native_init_module(); kvm_register_device_ops(&kvm_xive_ops, KVM_DEV_TYPE_XICS);+ kvm_register_device_ops(&kvm_xive_native_ops,+ KVM_DEV_TYPE_XIVE);
I think we want tighter conditions on initializing the xive_native
stuff and creating the xive device class. We could have
xive_enabled() returning true in a guest, and this code will get
called both by PR KVM and HV KVM (and HV KVM no longer implies that we
are running bare metal).
Ah yes, I agree. I haven't addressed at all the nested flavor. I have
some questions about this that I will ask in summary email you sent.
Thanks,
C.
quoted
@@ -1050,8 +1053,10 @@ static int kvmppc_book3s_init(void) static void kvmppc_book3s_exit(void) { #ifdef CONFIG_KVM_XICS- if (xive_enabled())+ if (xive_enabled()) { kvmppc_xive_exit_module();+ kvmppc_xive_native_exit_module();
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-23 16:58:06
On 1/22/19 6:14 AM, Paul Mackerras wrote:
On Mon, Jan 07, 2019 at 07:43:20PM +0100, Cédric Le Goater wrote:
quoted
The ESB MMIO region controls the interrupt sources of the guest. QEMU
will query an fd (GET_ESB_FD ioctl) and map this region at a specific
address for the guest to use. The guest will obtain this information
using the H_INT_GET_SOURCE_INFO hcall. To inform KVM of the address
setting used by QEMU, add a VC_BASE control to the KVM XIVE device
This needs a little more explanation. I *think* the only way this
gets used is that it gets returned to the guest by the new
hypercalls. If that is indeed the case it would be useful to mention
that in the patch description, because otherwise taking a value that
userspace provides and which looks like it is an address, and not
doing any validation on it, looks a bit scary.
I think we have solved this problem in another email thread.
The H_INT_GET_SOURCE_INFO hcall does not need to be implemented in KVM
as all the source information should already be available in QEMU. In
that case, there is no need to inform KVM of where the ESB pages are
mapped in the guest address space. So we don't need that extra control
on the KVM device. This is good news.
Thanks,
C.
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-23 16:59:31
On 1/22/19 6:09 AM, Paul Mackerras wrote:
On Mon, Jan 07, 2019 at 07:43:18PM +0100, Cédric Le Goater wrote:
quoted
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@kaod.org>
---
arch/powerpc/include/uapi/asm/kvm.h | 4 ++
arch/powerpc/kvm/book3s_xive_native.c | 97 +++++++++++++++++++++++++++
2 files changed, 101 insertions(+)
@@ -153,6 +153,85 @@ int kvmppc_xive_native_connect_vcpu(struct kvm_device *dev,returnrc;}+staticintxive_native_esb_fault(structvm_fault*vmf)+{+structvm_area_struct*vma=vmf->vma;+structkvmppc_xive*xive=vma->vm_file->private_data;+structkvmppc_xive_src_block*sb;+structkvmppc_xive_irq_state*state;+structxive_irq_data*xd;+u32hw_num;+u16src;+u64page;+unsignedlongirq;++/*+*Linux/KVMusesatwopagesESBsetting,onefortriggerand+*oneforEOI+*/+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)?
yes. It should. The SIGBUS is enough to know what's going on.
quoted
+ 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.
Not having an ESB page (trigger or EOI) implies that the xive_irq_data
for the source is bogus. This probably deserves a WARN().
Thanks,
C.
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-23 18:48:50
On 1/22/19 6:42 AM, Paul Mackerras wrote:
On Mon, Jan 07, 2019 at 08:10:06PM +0100, Cédric Le Goater wrote:
quoted
This will be used to destroy the KVM XICS or XIVE device when the
sPAPR machine is reseted. When the VM boots, the CAS negotiation
process will determine which interrupt mode to use and the appropriate
KVM device will then be created.
What would be the consequence if we didn't destroy the device?
So, if we don't destroy the device, it would mean that we are
maintaining its availability under the KVM PPC structures, VM and
vCPUs, I think the changes would be significant to have two interrupt
devices unde the VM. We would also need a way to activate one or
the other depending on the interrupt mode chosen by CAS. In other
words, it's moving all the interrupt mode politics from QEMU to KVM.
It's possible of course but I would prefer to leave the ugly details
in QEMU.
Let's suppose now that we keep the device alive but disconnect the
presenters from it, and from the VM also. We would have an unused
device in the VM. We would need way to keep an handle on it (fd
certainly) and a KVM interface to soft reset a KVM device partially
initialized. That's one other option.
It seemed easier to do an hard reset : create/destroy.
The reason I ask is that we will have to be much more careful about
memory allocation lifetimes with this patch.
yes. bad refcounting will lead the host kernel to a crash.
Having KVM devices last
until the KVM instance is destroyed means that we generally avoid
use-after-free bugs. With this patch we will have to do a careful
analysis of the lifetime of the xive structures vs. possible accesses
on other threads to prove there are no use-after-free bugs.
For example, it is not sufficient to set any pointers in struct kvm or
struct kvm_vcpu that point into xive structures to NULL before freeing
the structures. There could be code on another CPU that has read the
pointer value before you set it to NULL and then goes and accesses it
after you have freed it. You need to prove that can't happen,
possibly using some sort of explicit synchronization that ensures that
no other CPU could still be accessing the structure at the time when
you free it. RCU can help with this, but in general means you need
RCU synchronization primitives (rcu_read_lock() etc.) at all the
places where you use the pointer, which I don't think you currently
have.
no. indeed. I have overlooked the synchronization aspect.
If there is a good fundamental reason why this can't happen, even
though you don't have explicit synchronization, then at a minimum you
need to explain that in the patch description, and ideally also in
code comments.
OK. I did leave that patch at the end for one reason. It needs more care.
Thanks,
C.
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-23 19:19:45
On 1/22/19 5:46 AM, Paul Mackerras wrote:
On Mon, Jan 07, 2019 at 07:43:12PM +0100, Cédric Le Goater wrote:
quoted
Hello,
On the POWER9 processor, the XIVE interrupt controller can control
interrupt sources using MMIO to trigger events, to EOI or to turn off
the sources. Priority management and interrupt acknowledgment is also
controlled by MMIO in the CPU presenter subengine.
PowerNV/baremetal Linux runs natively under XIVE but sPAPR guests need
special support from the hypervisor to do the same. This is called the
XIVE native exploitation mode and today, it can be activated under the
PowerPC Hypervisor, pHyp. However, Linux/KVM lacks XIVE native support
and still offers the old interrupt mode interface using a
XICS-over-XIVE glue which implements the XICS hcalls.
The following series is proposal to add the same support under KVM.
A new KVM device is introduced for the XIVE native exploitation
mode. It reuses most of the XICS-over-XIVE glue implementation
structures which are internal to KVM but has a completely different
interface. A set of Hypervisor calls configures the sources and the
event queues and from there, all control is done by the guest through
MMIOs.
These MMIO regions (ESB and TIMA) are exposed to guests in QEMU,
similarly to VFIO, and the associated VMAs are populated dynamically
with the appropriate pages using a fault handler. This is implemented
with a couple of KVM device ioctls.
On a POWER9 sPAPR machine, the Client Architecture Support (CAS)
negotiation process determines whether the guest operates with a
interrupt controller using the XICS legacy model, as found on POWER8,
or in XIVE exploitation mode. Which means that the KVM interrupt
device should be created at runtime, after the machine as started.
This requires extra KVM support to create/destroy KVM devices. The
last patches are an attempt to solve that problem.
Migration has its own specific needs. The patchset provides the
necessary routines to quiesce XIVE, to capture and restore the state
of the different structures used by KVM, OPAL and HW. Extra OPAL
support is required for these.
Thanks for the patchset. It mostly looks good, but there are some
more things we need to consider, and I think a v2 will be needed.
quoted
One general comment I have is that there are a lot of acronyms in this
code and you mostly seem to assume that people will know what they all
mean. It would make the code more readable if you provide the
expansion of the acronym on first use in a comment or whatever. For
example, one of the patches in this series talks about the "EAS"
Event Assignment Structure, a.k.a IVE (Interrupt Virtualization Entry)
All the names changed somewhere between XIVE v1 and XIVE v2. OPAL and
Linux should be adjusted ...
without ever expanding it in any comment or in the patch description,
and I have forgotten just at the moment what EAS stands for (I just
know that understanding the XIVE is not eas-y. :)
Another general comment is that you seem to have written all this
code assuming we are using HV KVM in a host running bare-metal.
Yes. I didn't look at the other configurations. I thought that we could
use the kernel_irqchip=off option to begin with. A couple of checks
are indeed missing.
However, we could be using PR KVM (either in a bare-metal host or in a
guest), or we could be doing nested HV KVM where we are using the
kvm_hv module inside a KVM guest and using special hypercalls for
controlling our guests.
Yes.
It would be good to talk a little about the nested support (offline
may be) to make sure that we are not missing some major interface that
would require a lot of change. If we need to prepare ground, I think
the timing is good.
The size of the IRQ number space might be a problem. It seems we
would need to increase it considerably to support multiple nested
guests. That said I haven't look much how nested is designed.
It would be perfectly acceptable for now to say that we don't yet
support XIVE exploitation in those scenarios, as long as we then make
sure that the new KVM capability reports false in those scenarios, and
any attempt to use the XIVE exploitation interfaces fails cleanly.
ok. That looks the best approach for now.
I don't see that either of those is true in the patch set as it
stands, so that is one area that needs to be fixed.
A third general comment is that the new KVM interfaces you have added
need to be documented in the files under Documentation/virtual/kvm.
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2019-01-23 21:27:23
On Wed, 2019-01-23 at 21:30 +1100, Paul Mackerras wrote:
quoted
Afaik bcs we change the mapping to point to the real HW irq ESB page
instead of the "IPI" that was there at VM init time.
So that makes it sound like there is a whole lot going on that hasn't
even been hinted at in the patch descriptions... It sounds like we
need a good description of how all this works and fits together
somewhere under Documentation/.
In any case we need much more informative patch descriptions. I
realize that it's all currently in Cedric's head, but I bet that in
two or three years' time when we come to try to debug something, it
won't be in anyone's head...
The main problem is understanding XIVE itself. It's not realistic to
ask Cedric to write a proper documentation for XIVE as part of the
patch series, but sadly IBM doesn't have a good one to provide either.
Ben.
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-24 08:49:40
On 1/23/19 10:25 PM, Benjamin Herrenschmidt wrote:
On Wed, 2019-01-23 at 21:30 +1100, Paul Mackerras wrote:
quoted
quoted
Afaik bcs we change the mapping to point to the real HW irq ESB page
instead of the "IPI" that was there at VM init time.
So that makes it sound like there is a whole lot going on that hasn't
even been hinted at in the patch descriptions... It sounds like we
need a good description of how all this works and fits together
somewhere under Documentation/.
In any case we need much more informative patch descriptions. I
realize that it's all currently in Cedric's head, but I bet that in
two or three years' time when we come to try to debug something, it
won't be in anyone's head...
The main problem is understanding XIVE itself. It's not realistic to
ask Cedric to write a proper documentation for XIVE as part of the
patch series, but sadly IBM doesn't have a good one to provide either.
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2019-01-25 22:39:44
On Wed, 2019-01-23 at 19:39 +0100, Cédric Le Goater wrote:
quoted
The reason I ask is that we will have to be much more careful about
memory allocation lifetimes with this patch.
yes. bad refcounting will lead the host kernel to a crash.
One way to alleviate that is to make sure this is only supported on
selected devices such as XICS via some flag or the presence of a
callback.
Cheers,
Ben.
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2019-01-25 22:41:14
On Wed, 2019-01-23 at 20:07 +0100, Cédric Le Goater wrote:
Event Assignment Structure, a.k.a IVE (Interrupt Virtualization Entry)
All the names changed somewhere between XIVE v1 and XIVE v2. OPAL and
Linux should be adjusted ...
All the names changed between the HW design and the "architecture"
document. The HW guys use the old names, the architecture the new
names, and Linux & OPAL mostly use the old ones because frankly the new
names suck big time.
It would be good to talk a little about the nested support (offline
may be) to make sure that we are not missing some major interface that
would require a lot of change. If we need to prepare ground, I think
the timing is good.
The size of the IRQ number space might be a problem. It seems we
would need to increase it considerably to support multiple nested
guests. That said I haven't look much how nested is designed.
The size of the VP space is a bigger concern. Even today. We really
need qemu to tell the max #cpu to KVM so we can allocate less of them.
As for nesting, I suggest for the foreseeable future we stick to XICS
emulation in nested guests.
Cheers,
Ben.
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2019-01-25 22:42:42
On Wed, 2019-01-23 at 21:26 +1100, Paul Mackerras wrote:
If H_INT_ESB is only used for LSIs, then is a guest going to be using
it at all?
*emulated* LSIs, ie LSIs coming from emulated devices. It will depends
in practice of what kind of emulated device you put in your guest. We
need that because under the hood, we send a XIVE MSI, so we need to be
notified of the EOI so we can resend if the emulated LSI is still
asserted.
My understanding was that with XIVE, only a small number
of interrupts that are to do with system management functions are
LSIs; all of the interrupts relating to PCI-e devices are MSIs. So do
we actually have a real high-frequency use case for LSIs in a guest?
For now I would prefer that you remove all the real-mode hcall
handlers. We can add them later if we get performance data showing
that they are needed.
Regarding whether or not to have a given hcall handler in the kernel
at all - if there is for example an hcall which is just called once
on guest startup, and its function is just to provide information to
the guest, and QEMU has that information, then why not have that hcall
implemented by QEMU? Are any of the hcalls like that?
For example, if H_INT_GET_SOURCE_INFO was implemented in QEMU, could
we then remove the VC_BASE thing from the xive device?
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2019-01-26 00:13:54
On Tue, 2019-01-22 at 16:23 +1100, Paul Mackerras wrote:
Which ones of these could be implemented in QEMU? Are there any that
can't possibly be implemented in QEMU because they need to do things
that require calling internal interfaces that userspace doesn't have
access to?
Implementing them in qemu doesn't make a lot of sense. Qemu doesn't
have access to most of the XIVE HW state. There's a XIVE model for full
emulation but when using the real thing, almost none of it is visible.
A lot of those hcalls effectively turn into OPAL calls.
How often do we expect each of these hypercalls to be called?
It depends, I can't tell for AIX. For Linux, not often with one
exception: H_INT_ESB which will be used whenever you EOI an emulated
LSI.
.../...
Why do we need to provide real-mode versions of these hypercall
handlers? I thought these hypercalls would only get called
infrequently, and in any case certainly much less frequently than once
per interrupt delivered. If they are infrequent, then let's leave out
the real-mode version and just handle them in book3s_hv.c.
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-26 10:53:45
Was there a crashing.org shutdown ?
Received: from gate.crashing.org (gate.crashing.org [63.228.1.57])
by in5.mail.ovh.net (Postfix) with ESMTPS id 43mYnj0nrlz1N7KC
for [off-list ref]; Fri, 25 Jan 2019 22:38:00 +0000 (UTC)
Received: from localhost (localhost.localdomain [127.0.0.1])
by gate.crashing.org (8.14.1/8.14.1) with ESMTP id x0NLZf4K021092;
Wed, 23 Jan 2019 15:35:43 -0600
On 1/23/19 10:35 PM, Benjamin Herrenschmidt wrote:
On Wed, 2019-01-23 at 20:07 +0100, Cédric Le Goater wrote:
quoted
Event Assignment Structure, a.k.a IVE (Interrupt Virtualization Entry)
All the names changed somewhere between XIVE v1 and XIVE v2. OPAL and
Linux should be adjusted ...
All the names changed between the HW design and the "architecture"
document. The HW guys use the old names, the architecture the new
names, and Linux & OPAL mostly use the old ones because frankly the new
names suck big time.
Well, It does not make XIVE any clearer ... I did prefer the v1 names
but there was some naming overlap in the concepts.
quoted
It would be good to talk a little about the nested support (offline
may be) to make sure that we are not missing some major interface that
would require a lot of change. If we need to prepare ground, I think
the timing is good.
The size of the IRQ number space might be a problem. It seems we
would need to increase it considerably to support multiple nested
guests. That said I haven't look much how nested is designed.
The size of the VP space is a bigger concern. Even today. We really
need qemu to tell the max #cpu to KVM so we can allocate less of them.
ah yes. we would also need to reduce the number of available priorities
per CPU to have more EQ descriptors available if I recall well.
As for nesting, I suggest for the foreseeable future we stick to XICS
emulation in nested guests.
ok. so no kernel_irqchip at all. hmm.
I was wondering how possible it was to have L2 initialize the underlying
OPAL structures in the L0 hypervisor. May be with a sort of proxy hcall
which would perform the initialization in QEMU L1 on behalf of L2.
Cheers,
C.
From: Paul Mackerras <hidden> Date: 2019-01-28 04:45:37
On Thu, Jan 24, 2019 at 08:25:15AM +1100, Benjamin Herrenschmidt wrote:
On Wed, 2019-01-23 at 21:30 +1100, Paul Mackerras wrote:
quoted
quoted
Afaik bcs we change the mapping to point to the real HW irq ESB page
instead of the "IPI" that was there at VM init time.
So that makes it sound like there is a whole lot going on that hasn't
even been hinted at in the patch descriptions... It sounds like we
need a good description of how all this works and fits together
somewhere under Documentation/.
In any case we need much more informative patch descriptions. I
realize that it's all currently in Cedric's head, but I bet that in
two or three years' time when we come to try to debug something, it
won't be in anyone's head...
The main problem is understanding XIVE itself. It's not realistic to
ask Cedric to write a proper documentation for XIVE as part of the
patch series, but sadly IBM doesn't have a good one to provide either.
There are: (a) the XIVE hardware, (b) the definition of the XIVE
hypercalls that guests use, and (c) the design decisions around how to
implement that hypercall interface. We need to get (b) published
somehow, but it is mostly (c) that I would expect the patch
descriptions to explain.
It sounds like there will be a mapping to userspace where the pages
can sometimes point to an IPI page and sometimes point to a real HW
irq ESB page. That is, the same guest "hardware" irq number sometimes
refers to a software-generated interrupt (what you called an "IPI"
above) and sometimes to a hardware-generated interrupt. That fact,
the reason why it is so and the consequences all need to be explained
somewhere. They are really not obvious and I don't believe they are
part of either the XIVE hardware spec or the XIVE hypercall spec.
Paul.
From: Paul Mackerras <hidden> Date: 2019-01-28 05:52:43
On Wed, Jan 23, 2019 at 08:07:33PM +0100, Cédric Le Goater wrote:
On 1/22/19 5:46 AM, Paul Mackerras wrote:
quoted
On Mon, Jan 07, 2019 at 07:43:12PM +0100, Cédric Le Goater wrote:
quoted
Hello,
On the POWER9 processor, the XIVE interrupt controller can control
interrupt sources using MMIO to trigger events, to EOI or to turn off
the sources. Priority management and interrupt acknowledgment is also
controlled by MMIO in the CPU presenter subengine.
PowerNV/baremetal Linux runs natively under XIVE but sPAPR guests need
special support from the hypervisor to do the same. This is called the
XIVE native exploitation mode and today, it can be activated under the
PowerPC Hypervisor, pHyp. However, Linux/KVM lacks XIVE native support
and still offers the old interrupt mode interface using a
XICS-over-XIVE glue which implements the XICS hcalls.
The following series is proposal to add the same support under KVM.
A new KVM device is introduced for the XIVE native exploitation
mode. It reuses most of the XICS-over-XIVE glue implementation
structures which are internal to KVM but has a completely different
interface. A set of Hypervisor calls configures the sources and the
event queues and from there, all control is done by the guest through
MMIOs.
These MMIO regions (ESB and TIMA) are exposed to guests in QEMU,
similarly to VFIO, and the associated VMAs are populated dynamically
with the appropriate pages using a fault handler. This is implemented
with a couple of KVM device ioctls.
On a POWER9 sPAPR machine, the Client Architecture Support (CAS)
negotiation process determines whether the guest operates with a
interrupt controller using the XICS legacy model, as found on POWER8,
or in XIVE exploitation mode. Which means that the KVM interrupt
device should be created at runtime, after the machine as started.
This requires extra KVM support to create/destroy KVM devices. The
last patches are an attempt to solve that problem.
Migration has its own specific needs. The patchset provides the
necessary routines to quiesce XIVE, to capture and restore the state
of the different structures used by KVM, OPAL and HW. Extra OPAL
support is required for these.
Thanks for the patchset. It mostly looks good, but there are some
more things we need to consider, and I think a v2 will be needed.
quoted
One general comment I have is that there are a lot of acronyms in this
code and you mostly seem to assume that people will know what they all
mean. It would make the code more readable if you provide the
expansion of the acronym on first use in a comment or whatever. For
example, one of the patches in this series talks about the "EAS"
Event Assignment Structure, a.k.a IVE (Interrupt Virtualization Entry)
All the names changed somewhere between XIVE v1 and XIVE v2. OPAL and
Linux should be adjusted ...
quoted
without ever expanding it in any comment or in the patch description,
and I have forgotten just at the moment what EAS stands for (I just
know that understanding the XIVE is not eas-y. :)
Another general comment is that you seem to have written all this
code assuming we are using HV KVM in a host running bare-metal.
Yes. I didn't look at the other configurations. I thought that we could
use the kernel_irqchip=off option to begin with. A couple of checks
are indeed missing.
Using kernel_irqchip=off would mean that we would not be able to use
the in-kernel XICS emulation, which would have a performance impact.
We need an explicit capability for XIVE exploitation that can be
enabled or disabled on the qemu command line, so that we can enforce a
uniform set of capabilities across all the hosts in a migration
domain. And it's no good to say we have the capability when all
attempts to use it will fail. Therefore the kernel needs to say that
it doesn't have the capability in a PR KVM guest or in a nested HV
guest.
quoted
However, we could be using PR KVM (either in a bare-metal host or in a
guest), or we could be doing nested HV KVM where we are using the
kvm_hv module inside a KVM guest and using special hypercalls for
controlling our guests.
Yes.
It would be good to talk a little about the nested support (offline
may be) to make sure that we are not missing some major interface that
would require a lot of change. If we need to prepare ground, I think
the timing is good.
The size of the IRQ number space might be a problem. It seems we
would need to increase it considerably to support multiple nested
guests. That said I haven't look much how nested is designed.
The current design of nested HV is that the entire non-volatile state
of all the nested guests is encapsulated within the state and
resources of the L1 hypervisor. That means that if the L1 hypervisor
gets migrated, all of its guests go across inside it and there is no
extra state that L0 needs to be aware of. That would imply that the
VP number space for the nested guests would need to come from within
the VP number space for L1; but the amount of VP space we allocate to
each guest doesn't seem to be large enough for that to be practical.
quoted
It would be perfectly acceptable for now to say that we don't yet
support XIVE exploitation in those scenarios, as long as we then make
sure that the new KVM capability reports false in those scenarios, and
any attempt to use the XIVE exploitation interfaces fails cleanly.
ok. That looks the best approach for now.
quoted
I don't see that either of those is true in the patch set as it
stands, so that is one area that needs to be fixed.
A third general comment is that the new KVM interfaces you have added
need to be documented in the files under Documentation/virtual/kvm.
From: Paul Mackerras <hidden> Date: 2019-01-28 06:15:33
On Wed, Jan 23, 2019 at 12:07:19PM +0100, Cédric Le Goater wrote:
On 1/23/19 11:30 AM, Paul Mackerras wrote:
quoted
On Wed, Jan 23, 2019 at 05:45:24PM +1100, Benjamin Herrenschmidt wrote:
quoted
On Tue, 2019-01-22 at 16:26 +1100, Paul Mackerras wrote:
quoted
On Mon, Jan 07, 2019 at 08:10:05PM +0100, Cédric Le Goater wrote:
quoted
Clear the ESB pages from the VMA of the IRQ being pass through to the
guest and let the fault handler repopulate the VMA when the ESB pages
are accessed for an EOI or for a trigger.
Why do we want to do this?
I don't see any possible advantage to removing the PTEs from the
userspace mapping. You'll need to explain further.
Afaik bcs we change the mapping to point to the real HW irq ESB page
instead of the "IPI" that was there at VM init time.
yes exactly. You need to clean up the pages each time.
quoted
So that makes it sound like there is a whole lot going on that hasn't
even been hinted at in the patch descriptions... It sounds like we
need a good description of how all this works and fits together
somewhere under Documentation/.
OK. I have started doing so for the models merged in QEMU but not yet
for KVM. I will work on it.
quoted
In any case we need much more informative patch descriptions. I
realize that it's all currently in Cedric's head, but I bet that in
two or three years' time when we come to try to debug something, it
won't be in anyone's head...
I agree.
So, storing the ESB VMA under the KVM device is not shocking anyone ?
Actually, now that I think of it, why can't userspace (QEMU) manage
this using mmap()? Based on what Ben has said, I assume there would
be a pair of pages for each interrupt that a PCI pass-through device
has. Would we end up with too many VMAs if we just used mmap() to
change the mappings from the software-generated pages to the
hardware-generated interrupt pages? Are the necessary pages for a PCI
passthrough device contiguous in both host real space and guest real
space? If so we'd only need one mmap() for all the device's interrupt
pages.
Paul.
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-28 17:55:31
On 1/22/19 6:05 AM, Paul Mackerras wrote:
On Mon, Jan 07, 2019 at 07:43:17PM +0100, Cédric Le Goater wrote:
quoted
This is the basic framework for the new KVM device supporting the XIVE
native exploitation mode. The user interface exposes a new capability
and a new KVM device to be used by QEMU.
[snip]
quoted
@@ -1039,7 +1039,10 @@ static int kvmppc_book3s_init(void) #ifdef CONFIG_KVM_XIVE if (xive_enabled()) { kvmppc_xive_init_module();+ kvmppc_xive_native_init_module(); kvm_register_device_ops(&kvm_xive_ops, KVM_DEV_TYPE_XICS);+ kvm_register_device_ops(&kvm_xive_native_ops,+ KVM_DEV_TYPE_XIVE);
I think we want tighter conditions on initializing the xive_native
stuff and creating the xive device class. We could have
xive_enabled() returning true in a guest, and this code will get
called both by PR KVM and HV KVM (and HV KVM no longer implies that we
are running bare metal).
So yes, I gave nested a try with kernel_irqchip=on and the nested hypervisor
(L1) obviously crashes trying to call OPAL. I have tighten the test with :
if (xive_enabled() && !kvmhv_on_pseries()) {
for now.
As this is a problem today in 5.0.x, I will send a patch for it if you think
it is correct. I don't think we should bother taking care of the PR case
on P9. Should we ?
Thanks,
C.
quoted
@@ -1050,8 +1053,10 @@ static int kvmppc_book3s_init(void) static void kvmppc_book3s_exit(void) { #ifdef CONFIG_KVM_XICS- if (xive_enabled())+ if (xive_enabled()) { kvmppc_xive_exit_module();+ kvmppc_xive_native_exit_module();
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-28 19:45:41
On 1/28/19 7:13 AM, Paul Mackerras wrote:
On Wed, Jan 23, 2019 at 12:07:19PM +0100, Cédric Le Goater wrote:
quoted
On 1/23/19 11:30 AM, Paul Mackerras wrote:
quoted
On Wed, Jan 23, 2019 at 05:45:24PM +1100, Benjamin Herrenschmidt wrote:
quoted
On Tue, 2019-01-22 at 16:26 +1100, Paul Mackerras wrote:
quoted
On Mon, Jan 07, 2019 at 08:10:05PM +0100, Cédric Le Goater wrote:
quoted
Clear the ESB pages from the VMA of the IRQ being pass through to the
guest and let the fault handler repopulate the VMA when the ESB pages
are accessed for an EOI or for a trigger.
Why do we want to do this?
I don't see any possible advantage to removing the PTEs from the
userspace mapping. You'll need to explain further.
Afaik bcs we change the mapping to point to the real HW irq ESB page
instead of the "IPI" that was there at VM init time.
yes exactly. You need to clean up the pages each time.
quoted
So that makes it sound like there is a whole lot going on that hasn't
even been hinted at in the patch descriptions... It sounds like we
need a good description of how all this works and fits together
somewhere under Documentation/.
OK. I have started doing so for the models merged in QEMU but not yet
for KVM. I will work on it.
quoted
In any case we need much more informative patch descriptions. I
realize that it's all currently in Cedric's head, but I bet that in
two or three years' time when we come to try to debug something, it
won't be in anyone's head...
I agree.
So, storing the ESB VMA under the KVM device is not shocking anyone ?
Actually, now that I think of it, why can't userspace (QEMU) manage
this using mmap()? Based on what Ben has said, I assume there would
be a pair of pages for each interrupt that a PCI pass-through device
has.
Yes. there is a pair of ESB pages per IRQ number.
Would we end up with too many VMAs if we just used mmap() to
change the mappings from the software-generated pages to the
hardware-generated interrupt pages?
The sPAPR IRQ number space is 0x8000 wide now. The first 4K are
dedicated to CPU IPIs and the remaining 4K are for devices. We can
extend the last range if needed as these are for MSIs. Dynamic
extensions under KVM should work also.
This to say that we have with 8K x 2 (trigger+EOI) pages. This is a
lot of mmap(), too much. Also the KVM model needs to be compatible
with the QEMU emulated one and it was simpler to have one overall
memory region for the IPI ESBs, one for the END ESBs (if we support
that one day) and one for the TIMA.
Are the necessary pages for a PCI
passthrough device contiguous in both host real space
They should as they are the PHB4 ESBs.
and guest real space ?
also. That's how we organized the mapping.
If so we'd only need one mmap() for all the device's interrupt
pages.
Ah. So we would want to make a special case for the passthrough
device and have a mmap() and a memory region for its ESBs. Hmm.
Wouldn't that require to hot plug a memory region under the guest ?
which means that we need to provision an address space/container
region for theses regions. What are the benefits ?
Is clearing the PTEs and repopulating the VMA unsafe ?
Thanks,
C.
From: Paul Mackerras <hidden> Date: 2019-01-29 02:46:45
On Mon, Jan 28, 2019 at 07:26:00PM +0100, Cédric Le Goater wrote:
On 1/28/19 7:13 AM, Paul Mackerras wrote:
quoted
Would we end up with too many VMAs if we just used mmap() to
change the mappings from the software-generated pages to the
hardware-generated interrupt pages?
The sPAPR IRQ number space is 0x8000 wide now. The first 4K are
dedicated to CPU IPIs and the remaining 4K are for devices. We can
Confused. You say the number space has 32768 entries but then imply
there are only 8K entries. Do you mean that the API allows for 15-bit
IRQ numbers but we are only making using of 8192 of them?
extend the last range if needed as these are for MSIs. Dynamic
extensions under KVM should work also.
This to say that we have with 8K x 2 (trigger+EOI) pages. This is a
lot of mmap(), too much. Also the KVM model needs to be compatible
I wasn't suggesting an mmap per IRQ, I meant that the bulk of the
space would be covered by a single mmap, overlaid by subsequent mmaps
where we need to map real device interrupts.
with the QEMU emulated one and it was simpler to have one overall
memory region for the IPI ESBs, one for the END ESBs (if we support
that one day) and one for the TIMA.
quoted
Are the necessary pages for a PCI
passthrough device contiguous in both host real space
They should as they are the PHB4 ESBs.
quoted
and guest real space ?
also. That's how we organized the mapping.
"How we organized the mapping" is a significant design decision that I
haven't seen documented anywhere, and is really needed for
understanding what's going on.
quoted
If so we'd only need one mmap() for all the device's interrupt
pages.
Ah. So we would want to make a special case for the passthrough
device and have a mmap() and a memory region for its ESBs. Hmm.
Wouldn't that require to hot plug a memory region under the guest ?
No; the way that a memory region works is that userspace can do
whatever disparate mappings it likes within the region on the user
process side, and the corresponding region of guest real address space
follows that automatically.
which means that we need to provision an address space/container
region for theses regions. What are the benefits ?
Is clearing the PTEs and repopulating the VMA unsafe ?
Explicitly unmapping parts of the VMA seems like the wrong way to do
it. If you have a device mmap where the device wants to change the
physical page underlying parts of the mapping, there should be a way
for it to do that explicitly (but I don't know off the top of my head
what the interface to do that is).
However, I still haven't seen a clear and concise explanation of what
is being changed, when, and why we need to do that.
Paul.
From: Paul Mackerras <hidden> Date: 2019-01-29 04:14:20
On Mon, Jan 28, 2019 at 07:26:00PM +0100, Cédric Le Goater wrote:
Is clearing the PTEs and repopulating the VMA unsafe ?
Actually, now that I come to think of it, there could be any number of
VMAs (well, up to almost 64k of them), since once you have a file
descriptor you can call mmap on it multiple times.
The more I think about it, the more I think that getting userspace to
manage the mappings with mmap() and munmap() is the right idea if it
can be made to work.
Paul.
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-29 14:04:12
On 1/28/19 5:43 AM, Paul Mackerras wrote:
On Thu, Jan 24, 2019 at 08:25:15AM +1100, Benjamin Herrenschmidt wrote:
quoted
On Wed, 2019-01-23 at 21:30 +1100, Paul Mackerras wrote:
quoted
quoted
Afaik bcs we change the mapping to point to the real HW irq ESB page
instead of the "IPI" that was there at VM init time.
So that makes it sound like there is a whole lot going on that hasn't
even been hinted at in the patch descriptions... It sounds like we
need a good description of how all this works and fits together
somewhere under Documentation/.
In any case we need much more informative patch descriptions. I
realize that it's all currently in Cedric's head, but I bet that in
two or three years' time when we come to try to debug something, it
won't be in anyone's head...
The main problem is understanding XIVE itself. It's not realistic to
ask Cedric to write a proper documentation for XIVE as part of the
patch series, but sadly IBM doesn't have a good one to provide either.
There are: (a) the XIVE hardware, (b) the definition of the XIVE
hypercalls that guests use, and (c) the design decisions around how to
implement that hypercall interface. We need to get (b) published
somehow, but it is mostly (c) that I would expect the patch
descriptions to explain.
It sounds like there will be a mapping to userspace where the pages
can sometimes point to an IPI page and sometimes point to a real HW
irq ESB page.
Just to be clear. In both cases, these pages are real HW ESB pages.
They are just attached to a different controller : the XIVE IC for
the IPIs and the PHB4 for the others.
That is, the same guest "hardware" irq number sometimes
refers to a software-generated interrupt (what you called an "IPI"
above) and sometimes to a hardware-generated interrupt. That fact,> the reason why it is so and the consequences all need to be explained
somewhere. They are really not obvious and I don't believe they are
part of either the XIVE hardware spec or the XIVE hypercall spec.
I tried to put the reasons behind the current approach in another
thread, not saying this is the correct one.
Thanks,
C.
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-29 14:09:20
On 1/29/19 3:45 AM, Paul Mackerras wrote:
On Mon, Jan 28, 2019 at 07:26:00PM +0100, Cédric Le Goater wrote:
quoted
On 1/28/19 7:13 AM, Paul Mackerras wrote:
quoted
Would we end up with too many VMAs if we just used mmap() to
change the mappings from the software-generated pages to the
hardware-generated interrupt pages?
The sPAPR IRQ number space is 0x8000 wide now. The first 4K are
dedicated to CPU IPIs and the remaining 4K are for devices. We can
Confused. You say the number space has 32768 entries but then imply
there are only 8K entries. Do you mean that the API allows for 15-bit
IRQ numbers but we are only making using of 8192 of them?
extend the last range if needed as these are for MSIs. Dynamic
extensions under KVM should work also.
This to say that we have with 8K x 2 (trigger+EOI) pages. This is a
lot of mmap(), too much. Also the KVM model needs to be compatible
I wasn't suggesting an mmap per IRQ, I meant that the bulk of the
space would be covered by a single mmap, overlaid by subsequent mmaps
where we need to map real device interrupts.
ok. The same fault handler could be used to populate the VMA with the
ESB pages.
But it would mean extra work on the QEMU side, which is not needed
with this patch.
quoted
with the QEMU emulated one and it was simpler to have one overall
memory region for the IPI ESBs, one for the END ESBs (if we support
that one day) and one for the TIMA.
quoted
Are the necessary pages for a PCI
passthrough device contiguous in both host real space
They should as they are the PHB4 ESBs.
quoted
and guest real space ?
also. That's how we organized the mapping.
"How we organized the mapping" is a significant design decision that I
haven't seen documented anywhere, and is really needed for
understanding what's going on.
OK. I will add comments on that. See below for some description.
There is nothing fancy, it's simply indexed with the interrupt number,
like for HW, or for the QEMU XIVE emulated model.
quoted
quoted
If so we'd only need one mmap() for all the device's interrupt
pages.
Ah. So we would want to make a special case for the passthrough
device and have a mmap() and a memory region for its ESBs. Hmm.
Wouldn't that require to hot plug a memory region under the guest ?
No; the way that a memory region works is that userspace can do
whatever disparate mappings it likes within the region on the user
process side, and the corresponding region of guest real address space
follows that automatically.
yes. I suppose this should work also for 'ram device' memory mappings.
So when the passthrough device is added to the guest, we would add a
new 'ram device' memory region for the device interrupt ESB pages
that would overlap with the initial guest ESB pages.
This is really a different approach.
quoted
which means that we need to provision an address space/container
region for theses regions. What are the benefits ?
Is clearing the PTEs and repopulating the VMA unsafe ?
Explicitly unmapping parts of the VMA seems like the wrong way to do
it. If you have a device mmap where the device wants to change the
physical page underlying parts of the mapping, there should be a way
for it to do that explicitly (but I don't know off the top of my head
what the interface to do that is).
However, I still haven't seen a clear and concise explanation of what
is being changed, when, and why we need to do that.
Yes. I agree on that. The problem is not very different from what we
have today with the XICS-over-XIVE glue in KVM. Let me try to explain.
The KVM XICS-over-XIVE device and the proposed KVM XIVE native device
implement an IRQ space for the guest using the generic IPI interrupts
of the XIVE IC controller. These interrupts are allocated at the OPAL
level and "mapped" into the guest IRQ number space in the range 0-0x1FFF.
Interrupt management is performed in the XIVE way: using loads and
stores on the addresses of the XIVE IPI interrupt ESB pages.
Both KVM devices share the same internal structure caching information
on the interrupts, among which the xive_irq_data struct containing the
addresses of the IPI ESB pages and an extra one in case of passthrough.
The later contains the addresses of the ESB pages of the underlying HW
controller interrupts, PHB4 in all cases for now.
A guest when running in the XICS legacy interrupt mode lets the KVM
XICS-over-XIVE device "handle" interrupt management, that is to perform
the loads and stores on the addresses of the ESB pages of the guest
interrupts.
However, when running in XIVE native exploitation mode, the KVM XIVE
native device exposes the interrupt ESB pages to the guest and lets
the guest perform directly the loads and stores.
The VMA exposing the ESB pages make use of a custom VM fault handler
which role is to populate the VMA with appropriate pages. When a fault
occurs, the guest IRQ number is deduced from the offset, and the ESB
pages of associated XIVE IPI interrupt are inserted in the VMA (using
the internal structure caching information on the interrupts).
Supporting device passthrough in the guest running in XIVE native
exploitation mode adds some extra refinements because the ESB pages
of a different HW controller (PHB4) need to be exposed to the guest
along with the initial IPI ESB pages of the XIVE IC controller. But
the overall mechanic is the same.
When the device HW irqs are mapped into or unmapped from the guest
IRQ number space, the passthru_irq helpers, kvmppc_xive_set_mapped()
and kvmppc_xive_clr_mapped(), are called to record or clear the
passthrough interrupt information and to perform the switch.
The approach taken by this patch is to clear the ESB pages of the
guest IRQ number being mapped and let the VM fault handler repopulate.
The handler will insert the ESB page corresponding to the HW interrupt
of the device being passed-through or the initial IPI ESB page if the
device is being removed.
Thanks,
C.
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-29 14:29:55
quoted
quoted
Another general comment is that you seem to have written all this
code assuming we are using HV KVM in a host running bare-metal.
Yes. I didn't look at the other configurations. I thought that we could
use the kernel_irqchip=off option to begin with. A couple of checks
are indeed missing.
Using kernel_irqchip=off would mean that we would not be able to use
the in-kernel XICS emulation, which would have a performance impact.
yes. But it is not supported today. Correct ?
We need an explicit capability for XIVE exploitation that can be
enabled or disabled on the qemu command line, so that we can enforce a
uniform set of capabilities across all the hosts in a migration
domain. And it's no good to say we have the capability when all
attempts to use it will fail. Therefore the kernel needs to say that
it doesn't have the capability in a PR KVM guest or in a nested HV
guest.
OK. I will work on adding a KVM_CAP_PPC_NESTED_IRQ_HV capability
for future use.
quoted
quoted
However, we could be using PR KVM (either in a bare-metal host or in a
guest), or we could be doing nested HV KVM where we are using the
kvm_hv module inside a KVM guest and using special hypercalls for
controlling our guests.
Yes.
It would be good to talk a little about the nested support (offline
may be) to make sure that we are not missing some major interface that
would require a lot of change. If we need to prepare ground, I think
the timing is good.
The size of the IRQ number space might be a problem. It seems we
would need to increase it considerably to support multiple nested
guests. That said I haven't look much how nested is designed.
The current design of nested HV is that the entire non-volatile state
of all the nested guests is encapsulated within the state and
resources of the L1 hypervisor. That means that if the L1 hypervisor
gets migrated, all of its guests go across inside it and there is no
extra state that L0 needs to be aware of. That would imply that the
VP number space for the nested guests would need to come from within
the VP number space for L1; but the amount of VP space we allocate to
each guest doesn't seem to be large enough for that to be practical.
If the KVM XIVE device had some information on the max number of CPUs
provisioned for the guest, we could optimize the VP allocation.
That might be a larger KVM topic though. There are some static limits
on the number of CPUs in QEMU and in KVM, which have no relation AFAICT.
Thanks,
C.
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-29 18:23:36
On 1/29/19 5:12 AM, Paul Mackerras wrote:
On Mon, Jan 28, 2019 at 07:26:00PM +0100, Cédric Le Goater wrote:
quoted
Is clearing the PTEs and repopulating the VMA unsafe ?
Actually, now that I come to think of it, there could be any number of
VMAs (well, up to almost 64k of them), since once you have a file
descriptor you can call mmap on it multiple times.
The more I think about it, the more I think that getting userspace to
manage the mappings with mmap() and munmap() is the right idea if it
can be made to work.
We might be able to mmap() and munmap() regions of ESB pages in the RTAS
call "ibm,change-msi". I think that's the right spot for it.
Thanks,
C.
From: Paul Mackerras <hidden> Date: 2019-01-30 06:22:48
On Tue, Jan 29, 2019 at 06:44:50PM +0100, Cédric Le Goater wrote:
On 1/29/19 5:12 AM, Paul Mackerras wrote:
quoted
On Mon, Jan 28, 2019 at 07:26:00PM +0100, Cédric Le Goater wrote:
quoted
Is clearing the PTEs and repopulating the VMA unsafe ?
Actually, now that I come to think of it, there could be any number of
VMAs (well, up to almost 64k of them), since once you have a file
descriptor you can call mmap on it multiple times.
The more I think about it, the more I think that getting userspace to
manage the mappings with mmap() and munmap() is the right idea if it
can be made to work.
We might be able to mmap() and munmap() regions of ESB pages in the RTAS
call "ibm,change-msi". I think that's the right spot for it.
I was thinking that the ESB pages should be mmapped for device
interrupts at VM startup or when a device is hot-plugged in, and
munmapped when the device is hot-removed. Maybe the mmap could be
done in conjunction with the KVM_IRQFD call?
What is the reasoning behind doing it in the ibm,change-msi call?
Paul.
From: Paul Mackerras <hidden> Date: 2019-01-30 06:24:23
On Tue, Jan 29, 2019 at 02:47:55PM +0100, Cédric Le Goater wrote:
On 1/29/19 3:45 AM, Paul Mackerras wrote:
quoted
On Mon, Jan 28, 2019 at 07:26:00PM +0100, Cédric Le Goater wrote:
quoted
On 1/28/19 7:13 AM, Paul Mackerras wrote:
quoted
Would we end up with too many VMAs if we just used mmap() to
change the mappings from the software-generated pages to the
hardware-generated interrupt pages?
The sPAPR IRQ number space is 0x8000 wide now. The first 4K are
dedicated to CPU IPIs and the remaining 4K are for devices. We can
Confused. You say the number space has 32768 entries but then imply
there are only 8K entries. Do you mean that the API allows for 15-bit
IRQ numbers but we are only making using of 8192 of them?
extend the last range if needed as these are for MSIs. Dynamic
extensions under KVM should work also.
This to say that we have with 8K x 2 (trigger+EOI) pages. This is a
lot of mmap(), too much. Also the KVM model needs to be compatible
I wasn't suggesting an mmap per IRQ, I meant that the bulk of the
space would be covered by a single mmap, overlaid by subsequent mmaps
where we need to map real device interrupts.
ok. The same fault handler could be used to populate the VMA with the
ESB pages.
But it would mean extra work on the QEMU side, which is not needed
with this patch.
Maybe, but just storing a single vma pointer in our private data is
not a feasible approach. First, you have no control on the lifetime
of the vma and thus this is a use-after-free waiting to happen, and
secondly, there could be multiple vmas that you need to worry about.
Userspace could do multiple mmaps, or could do mprotect or similar on
part of an existing mmap, which would split the vma for the mmap into
multiple vmas. You don't get notified about munmap either as far as I
can tell, so the vma is liable to go away. And it doesn't matter if
QEMU would never do such things; if userspace can provoke a
use-after-free in the kernel using KVM then someone will write a
specially crafted user program to do that.
So we either solve it in userspace, or we have to write and maintain
complex kernel code with deep links into the MM subsystem. I'd much
rather solve it in userspace.
quoted
quoted
with the QEMU emulated one and it was simpler to have one overall
memory region for the IPI ESBs, one for the END ESBs (if we support
that one day) and one for the TIMA.
quoted
Are the necessary pages for a PCI
passthrough device contiguous in both host real space
They should as they are the PHB4 ESBs.
quoted
and guest real space ?
also. That's how we organized the mapping.
"How we organized the mapping" is a significant design decision that I
haven't seen documented anywhere, and is really needed for
understanding what's going on.
OK. I will add comments on that. See below for some description.
There is nothing fancy, it's simply indexed with the interrupt number,
like for HW, or for the QEMU XIVE emulated model.
quoted
quoted
quoted
If so we'd only need one mmap() for all the device's interrupt
pages.
Ah. So we would want to make a special case for the passthrough
device and have a mmap() and a memory region for its ESBs. Hmm.
Wouldn't that require to hot plug a memory region under the guest ?
No; the way that a memory region works is that userspace can do
whatever disparate mappings it likes within the region on the user
process side, and the corresponding region of guest real address space
follows that automatically.
yes. I suppose this should work also for 'ram device' memory mappings.
So when the passthrough device is added to the guest, we would add a
new 'ram device' memory region for the device interrupt ESB pages
that would overlap with the initial guest ESB pages.
Not knowing the QEMU internals all that well, I don't at all
understand why a new ram device is necesssary. I would see it as a
single virtual area mapping the ESB pages of guest hwirqs that are in
use, and we manage those mappings with mmap and munmap.
This is really a different approach.
quoted
quoted
which means that we need to provision an address space/container
region for theses regions. What are the benefits ?
Is clearing the PTEs and repopulating the VMA unsafe ?
Explicitly unmapping parts of the VMA seems like the wrong way to do
it. If you have a device mmap where the device wants to change the
physical page underlying parts of the mapping, there should be a way
for it to do that explicitly (but I don't know off the top of my head
what the interface to do that is).
However, I still haven't seen a clear and concise explanation of what
is being changed, when, and why we need to do that.
Yes. I agree on that. The problem is not very different from what we
have today with the XICS-over-XIVE glue in KVM. Let me try to explain.
The KVM XICS-over-XIVE device and the proposed KVM XIVE native device
implement an IRQ space for the guest using the generic IPI interrupts
of the XIVE IC controller. These interrupts are allocated at the OPAL
level and "mapped" into the guest IRQ number space in the range 0-0x1FFF.
Interrupt management is performed in the XIVE way: using loads and
stores on the addresses of the XIVE IPI interrupt ESB pages.
Both KVM devices share the same internal structure caching information
on the interrupts, among which the xive_irq_data struct containing the
addresses of the IPI ESB pages and an extra one in case of passthrough.
The later contains the addresses of the ESB pages of the underlying HW
controller interrupts, PHB4 in all cases for now.
A guest when running in the XICS legacy interrupt mode lets the KVM
XICS-over-XIVE device "handle" interrupt management, that is to perform
the loads and stores on the addresses of the ESB pages of the guest
interrupts.
However, when running in XIVE native exploitation mode, the KVM XIVE
native device exposes the interrupt ESB pages to the guest and lets
the guest perform directly the loads and stores.
The VMA exposing the ESB pages make use of a custom VM fault handler
which role is to populate the VMA with appropriate pages. When a fault
occurs, the guest IRQ number is deduced from the offset, and the ESB
pages of associated XIVE IPI interrupt are inserted in the VMA (using
the internal structure caching information on the interrupts).
Supporting device passthrough in the guest running in XIVE native
exploitation mode adds some extra refinements because the ESB pages
of a different HW controller (PHB4) need to be exposed to the guest
along with the initial IPI ESB pages of the XIVE IC controller. But
the overall mechanic is the same.
When the device HW irqs are mapped into or unmapped from the guest
IRQ number space, the passthru_irq helpers, kvmppc_xive_set_mapped()
and kvmppc_xive_clr_mapped(), are called to record or clear the
passthrough interrupt information and to perform the switch.
The approach taken by this patch is to clear the ESB pages of the
guest IRQ number being mapped and let the VM fault handler repopulate.
The handler will insert the ESB page corresponding to the HW interrupt
of the device being passed-through or the initial IPI ESB page if the
device is being removed.
From: Paul Mackerras <hidden> Date: 2019-01-30 06:26:07
On Tue, Jan 29, 2019 at 02:51:05PM +0100, Cédric Le Goater wrote:
quoted
quoted
quoted
Another general comment is that you seem to have written all this
code assuming we are using HV KVM in a host running bare-metal.
Yes. I didn't look at the other configurations. I thought that we could
use the kernel_irqchip=off option to begin with. A couple of checks
are indeed missing.
Using kernel_irqchip=off would mean that we would not be able to use
the in-kernel XICS emulation, which would have a performance impact.
yes. But it is not supported today. Correct ?
Not correct, it has been working for years, and works in v5.0-rc1 (I
just tested it), at both L0 and L1.
quoted
We need an explicit capability for XIVE exploitation that can be
enabled or disabled on the qemu command line, so that we can enforce a
uniform set of capabilities across all the hosts in a migration
domain. And it's no good to say we have the capability when all
attempts to use it will fail. Therefore the kernel needs to say that
it doesn't have the capability in a PR KVM guest or in a nested HV
guest.
OK. I will work on adding a KVM_CAP_PPC_NESTED_IRQ_HV capability
for future use.
That's not what I meant. Why do we need that? I meant that querying
the new KVM_CAP_PPC_IRQ_XIVE capability should return 0 if we are in a
guest. It should only return 1 if we are running bare-metal on a P9.
quoted
quoted
quoted
However, we could be using PR KVM (either in a bare-metal host or in a
guest), or we could be doing nested HV KVM where we are using the
kvm_hv module inside a KVM guest and using special hypercalls for
controlling our guests.
Yes.
It would be good to talk a little about the nested support (offline
may be) to make sure that we are not missing some major interface that
would require a lot of change. If we need to prepare ground, I think
the timing is good.
The size of the IRQ number space might be a problem. It seems we
would need to increase it considerably to support multiple nested
guests. That said I haven't look much how nested is designed.
The current design of nested HV is that the entire non-volatile state
of all the nested guests is encapsulated within the state and
resources of the L1 hypervisor. That means that if the L1 hypervisor
gets migrated, all of its guests go across inside it and there is no
extra state that L0 needs to be aware of. That would imply that the
VP number space for the nested guests would need to come from within
the VP number space for L1; but the amount of VP space we allocate to
each guest doesn't seem to be large enough for that to be practical.
If the KVM XIVE device had some information on the max number of CPUs
provisioned for the guest, we could optimize the VP allocation.
The problem is that we might have 1000 guests running under L0, or we
might have 1 guest running under L0 and 1000 guests running under it,
and we have no way to know which situation to optimize for at the
point where an L1 guest starts. If we had an enormous VP space then
we could just give each L1 guest a large amount of VP space and solve
it that way; but we don't.
Paul.
From: Paul Mackerras <hidden> Date: 2019-01-30 06:27:31
On Mon, Jan 28, 2019 at 06:35:34PM +0100, Cédric Le Goater wrote:
On 1/22/19 6:05 AM, Paul Mackerras wrote:
quoted
On Mon, Jan 07, 2019 at 07:43:17PM +0100, Cédric Le Goater wrote:
quoted
This is the basic framework for the new KVM device supporting the XIVE
native exploitation mode. The user interface exposes a new capability
and a new KVM device to be used by QEMU.
[snip]
quoted
@@ -1039,7 +1039,10 @@ static int kvmppc_book3s_init(void) #ifdef CONFIG_KVM_XIVE if (xive_enabled()) { kvmppc_xive_init_module();+ kvmppc_xive_native_init_module(); kvm_register_device_ops(&kvm_xive_ops, KVM_DEV_TYPE_XICS);+ kvm_register_device_ops(&kvm_xive_native_ops,+ KVM_DEV_TYPE_XIVE);
I think we want tighter conditions on initializing the xive_native
stuff and creating the xive device class. We could have
xive_enabled() returning true in a guest, and this code will get
called both by PR KVM and HV KVM (and HV KVM no longer implies that we
are running bare metal).
So yes, I gave nested a try with kernel_irqchip=on and the nested hypervisor
(L1) obviously crashes trying to call OPAL. I have tighten the test with :
if (xive_enabled() && !kvmhv_on_pseries()) {
for now.
As this is a problem today in 5.0.x, I will send a patch for it if you think
How do you mean this is a problem today in 5.0? I just tried 5.0-rc1
with kernel_irqchip=on in a nested guest and it works just fine. What
exactly did you test?
it is correct. I don't think we should bother taking care of the PR case
on P9. Should we ?
We do need to take care of PR KVM on P9, since it is the only form of
nested KVM that works inside a host in HPT mode.
Paul.
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-30 07:03:21
On 1/30/19 5:29 AM, Paul Mackerras wrote:
On Mon, Jan 28, 2019 at 06:35:34PM +0100, Cédric Le Goater wrote:
quoted
On 1/22/19 6:05 AM, Paul Mackerras wrote:
quoted
On Mon, Jan 07, 2019 at 07:43:17PM +0100, Cédric Le Goater wrote:
quoted
This is the basic framework for the new KVM device supporting the XIVE
native exploitation mode. The user interface exposes a new capability
and a new KVM device to be used by QEMU.
[snip]
quoted
@@ -1039,7 +1039,10 @@ static int kvmppc_book3s_init(void) #ifdef CONFIG_KVM_XIVE if (xive_enabled()) { kvmppc_xive_init_module();+ kvmppc_xive_native_init_module(); kvm_register_device_ops(&kvm_xive_ops, KVM_DEV_TYPE_XICS);+ kvm_register_device_ops(&kvm_xive_native_ops,+ KVM_DEV_TYPE_XIVE);
I think we want tighter conditions on initializing the xive_native
stuff and creating the xive device class. We could have
xive_enabled() returning true in a guest, and this code will get
called both by PR KVM and HV KVM (and HV KVM no longer implies that we
are running bare metal).
So yes, I gave nested a try with kernel_irqchip=on and the nested hypervisor
(L1) obviously crashes trying to call OPAL. I have tighten the test with :
if (xive_enabled() && !kvmhv_on_pseries()) {
for now.
As this is a problem today in 5.0.x, I will send a patch for it if you think
How do you mean this is a problem today in 5.0? I just tried 5.0-rc1
with kernel_irqchip=on in a nested guest and it works just fine. What
exactly did you test?
L0: Linux 5.0.0-rc3 (+ KVM HV)
L1: QEMU pseries-4.0 (kernel_irqchip=on) - Linux 5.0.0-rc3 (+ KVM HV)
L2: QEMU pseries-4.0 (kernel_irqchip=on) - Linux 5.0.0-rc3
L1 crashes when L2 starts and tries to initialize the KVM IRQ device as
it does an OPAL call and its running under SLOF. See below.
I don't understand how L2 can work with kernel_irqchip=on. Could you
please explain ?
quoted
it is correct. I don't think we should bother taking care of the PR case
on P9. Should we ?
We do need to take care of PR KVM on P9, since it is the only form of
nested KVM that works inside a host in HPT mode.
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-30 09:33:52
On 1/30/19 6:55 AM, Paul Mackerras wrote:
On Tue, Jan 29, 2019 at 06:44:50PM +0100, Cédric Le Goater wrote:
quoted
On 1/29/19 5:12 AM, Paul Mackerras wrote:
quoted
On Mon, Jan 28, 2019 at 07:26:00PM +0100, Cédric Le Goater wrote:
quoted
Is clearing the PTEs and repopulating the VMA unsafe ?
Actually, now that I come to think of it, there could be any number of
VMAs (well, up to almost 64k of them), since once you have a file
descriptor you can call mmap on it multiple times.
The more I think about it, the more I think that getting userspace to
manage the mappings with mmap() and munmap() is the right idea if it
can be made to work.
We might be able to mmap() and munmap() regions of ESB pages in the RTAS
call "ibm,change-msi". I think that's the right spot for it.
I was thinking that the ESB pages should be mmapped for device
interrupts at VM startup or when a device is hot-plugged in, and
munmapped when the device is hot-removed. Maybe the mmap could be
done in conjunction with the KVM_IRQFD call?
What is the reasoning behind doing it in the ibm,change-msi call?
Because when the device is plugged in, it has no interrupts. These are
allocated on demand only when the driver is loaded in the guest.
C.
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-30 15:44:57
On 1/30/19 6:40 AM, Paul Mackerras wrote:
On Tue, Jan 29, 2019 at 02:51:05PM +0100, Cédric Le Goater wrote:
quoted
quoted
quoted
quoted
Another general comment is that you seem to have written all this
code assuming we are using HV KVM in a host running bare-metal.
Yes. I didn't look at the other configurations. I thought that we could
use the kernel_irqchip=off option to begin with. A couple of checks
are indeed missing.
Using kernel_irqchip=off would mean that we would not be able to use
the in-kernel XICS emulation, which would have a performance impact.
yes. But it is not supported today. Correct ?
Not correct, it has been working for years, and works in v5.0-rc1 (I
just tested it), at both L0 and L1.
Please see other email for the test is did.
quoted
quoted
We need an explicit capability for XIVE exploitation that can be
enabled or disabled on the qemu command line, so that we can enforce a
uniform set of capabilities across all the hosts in a migration
domain. And it's no good to say we have the capability when all
attempts to use it will fail. Therefore the kernel needs to say that
it doesn't have the capability in a PR KVM guest or in a nested HV
guest.
OK. I will work on adding a KVM_CAP_PPC_NESTED_IRQ_HV capability
for future use.
That's not what I meant. Why do we need that? I meant that querying
the new KVM_CAP_PPC_IRQ_XIVE capability should return 0 if we are in a
guest. It should only return 1 if we are running bare-metal on a P9.
ok. I guess I need to understand first how the nested guest uses the
KVM IRQ device. That is a question in another email thread.
quoted
quoted
quoted
quoted
However, we could be using PR KVM (either in a bare-metal host or in a
guest), or we could be doing nested HV KVM where we are using the
kvm_hv module inside a KVM guest and using special hypercalls for
controlling our guests.
Yes.
It would be good to talk a little about the nested support (offline
may be) to make sure that we are not missing some major interface that
would require a lot of change. If we need to prepare ground, I think
the timing is good.
The size of the IRQ number space might be a problem. It seems we
would need to increase it considerably to support multiple nested
guests. That said I haven't look much how nested is designed.
The current design of nested HV is that the entire non-volatile state
of all the nested guests is encapsulated within the state and
resources of the L1 hypervisor. That means that if the L1 hypervisor
gets migrated, all of its guests go across inside it and there is no
extra state that L0 needs to be aware of. That would imply that the
VP number space for the nested guests would need to come from within
the VP number space for L1; but the amount of VP space we allocate to
each guest doesn't seem to be large enough for that to be practical.
If the KVM XIVE device had some information on the max number of CPUs
provisioned for the guest, we could optimize the VP allocation.
The problem is that we might have 1000 guests running under L0, or we
might have 1 guest running under L0 and 1000 guests running under it,
and we have no way to know which situation to optimize for at the
point where an L1 guest starts. If we had an enormous VP space then
we could just give each L1 guest a large amount of VP space and solve
it that way; but we don't.
There are some ideas to increase our VP space size. Using multiblock
per XIVE chip in skiboot is one I think. It's not an obvious change.
Also, XIVE2 will add more bits to the NVT index so we will be free
to allocate more at once when P10 is available.
On the same topic, may be we could move the VP allocator from skiboot
to KVM, allocate the full VP space at the KVM level and let KVM do
the VP segmentation.
Any how, I think that if we knew how much VPs we need to provision for
when the KVM XIVE device is created, we would make a better use of the
available space. Shouldn't we ?
Thanks,
C.
From: Cédric Le Goater <clg@kaod.org> Date: 2019-01-30 15:56:26
On 1/30/19 7:20 AM, Paul Mackerras wrote:
On Tue, Jan 29, 2019 at 02:47:55PM +0100, Cédric Le Goater wrote:
quoted
On 1/29/19 3:45 AM, Paul Mackerras wrote:
quoted
On Mon, Jan 28, 2019 at 07:26:00PM +0100, Cédric Le Goater wrote:
quoted
On 1/28/19 7:13 AM, Paul Mackerras wrote:
quoted
Would we end up with too many VMAs if we just used mmap() to
change the mappings from the software-generated pages to the
hardware-generated interrupt pages?
The sPAPR IRQ number space is 0x8000 wide now. The first 4K are
dedicated to CPU IPIs and the remaining 4K are for devices. We can
Confused. You say the number space has 32768 entries but then imply
there are only 8K entries. Do you mean that the API allows for 15-bit
IRQ numbers but we are only making using of 8192 of them?
extend the last range if needed as these are for MSIs. Dynamic
extensions under KVM should work also.
This to say that we have with 8K x 2 (trigger+EOI) pages. This is a
lot of mmap(), too much. Also the KVM model needs to be compatible
I wasn't suggesting an mmap per IRQ, I meant that the bulk of the
space would be covered by a single mmap, overlaid by subsequent mmaps
where we need to map real device interrupts.
ok. The same fault handler could be used to populate the VMA with the
ESB pages.
But it would mean extra work on the QEMU side, which is not needed
with this patch.
Maybe, but just storing a single vma pointer in our private data is
not a feasible approach. First, you have no control on the lifetime
of the vma and thus this is a use-after-free waiting to happen, and
secondly, there could be multiple vmas that you need to worry about.
I fully agree. That's why I was uncomfortable with the solution. There
are a few other drivers (GPUs if I recall) doing that but it feels wrong.
Userspace could do multiple mmaps, or could do mprotect or similar on
part of an existing mmap, which would split the vma for the mmap into
multiple vmas. You don't get notified about munmap either as far as I
can tell, so the vma is liable to go away.
yes ...
And it doesn't matter if
QEMU would never do such things; if userspace can provoke a
use-after-free in the kernel using KVM then someone will write a
specially crafted user program to do that.
So we either solve it in userspace, or we have to write and maintain
complex kernel code with deep links into the MM subsystem.
I'd much rather solve it in userspace.
OK, then. I have been reluctant doing so but it seems there are no
other in-kernel solution.
quoted
quoted
quoted
with the QEMU emulated one and it was simpler to have one overall
memory region for the IPI ESBs, one for the END ESBs (if we support
that one day) and one for the TIMA.
quoted
Are the necessary pages for a PCI
passthrough device contiguous in both host real space
They should as they are the PHB4 ESBs.
quoted
and guest real space ?
also. That's how we organized the mapping.
"How we organized the mapping" is a significant design decision that I
haven't seen documented anywhere, and is really needed for
understanding what's going on.
OK. I will add comments on that. See below for some description.
There is nothing fancy, it's simply indexed with the interrupt number,
like for HW, or for the QEMU XIVE emulated model.
quoted
quoted
quoted
If so we'd only need one mmap() for all the device's interrupt
pages.
Ah. So we would want to make a special case for the passthrough
device and have a mmap() and a memory region for its ESBs. Hmm.
Wouldn't that require to hot plug a memory region under the guest ?
No; the way that a memory region works is that userspace can do
whatever disparate mappings it likes within the region on the user
process side, and the corresponding region of guest real address space
follows that automatically.
yes. I suppose this should work also for 'ram device' memory mappings.
So when the passthrough device is added to the guest, we would add a
new 'ram device' memory region for the device interrupt ESB pages
that would overlap with the initial guest ESB pages.
Not knowing the QEMU internals all that well, I don't at all
understand why a new ram device is necesssary.
'ram device' memory regions are of a special type which is used to
directly map into the guest the result of a mmap() in QEMU.
This is how we propagate the XIVE ESB pages from HW (and the TIMA)
to the guest and the Linux kernel. It has other use with VFIO.
I would see it as a
single virtual area mapping the ESB pages of guest hwirqs that are in
use, and we manage those mappings with mmap and munmap.
Yes I think I understand the idea. I will give a try. I need to find the
right place to do so in QEMU. See other email thread.
quoted
This is really a different approach.
quoted
quoted
which means that we need to provision an address space/container
region for theses regions. What are the benefits ?
Is clearing the PTEs and repopulating the VMA unsafe ?
Explicitly unmapping parts of the VMA seems like the wrong way to do
it. If you have a device mmap where the device wants to change the
physical page underlying parts of the mapping, there should be a way
for it to do that explicitly (but I don't know off the top of my head
what the interface to do that is).
However, I still haven't seen a clear and concise explanation of what
is being changed, when, and why we need to do that.
Yes. I agree on that. The problem is not very different from what we
have today with the XICS-over-XIVE glue in KVM. Let me try to explain.
The KVM XICS-over-XIVE device and the proposed KVM XIVE native device
implement an IRQ space for the guest using the generic IPI interrupts
of the XIVE IC controller. These interrupts are allocated at the OPAL
level and "mapped" into the guest IRQ number space in the range 0-0x1FFF.
Interrupt management is performed in the XIVE way: using loads and
stores on the addresses of the XIVE IPI interrupt ESB pages.
Both KVM devices share the same internal structure caching information
on the interrupts, among which the xive_irq_data struct containing the
addresses of the IPI ESB pages and an extra one in case of passthrough.
The later contains the addresses of the ESB pages of the underlying HW
controller interrupts, PHB4 in all cases for now.
A guest when running in the XICS legacy interrupt mode lets the KVM
XICS-over-XIVE device "handle" interrupt management, that is to perform
the loads and stores on the addresses of the ESB pages of the guest
interrupts.
However, when running in XIVE native exploitation mode, the KVM XIVE
native device exposes the interrupt ESB pages to the guest and lets
the guest perform directly the loads and stores.
The VMA exposing the ESB pages make use of a custom VM fault handler
which role is to populate the VMA with appropriate pages. When a fault
occurs, the guest IRQ number is deduced from the offset, and the ESB
pages of associated XIVE IPI interrupt are inserted in the VMA (using
the internal structure caching information on the interrupts).
Supporting device passthrough in the guest running in XIVE native
exploitation mode adds some extra refinements because the ESB pages
of a different HW controller (PHB4) need to be exposed to the guest
along with the initial IPI ESB pages of the XIVE IC controller. But
the overall mechanic is the same.
When the device HW irqs are mapped into or unmapped from the guest
IRQ number space, the passthru_irq helpers, kvmppc_xive_set_mapped()
and kvmppc_xive_clr_mapped(), are called to record or clear the
passthrough interrupt information and to perform the switch.
The approach taken by this patch is to clear the ESB pages of the
guest IRQ number being mapped and let the VM fault handler repopulate.
The handler will insert the ESB page corresponding to the HW interrupt
of the device being passed-through or the initial IPI ESB page if the
device is being removed.
From: Paul Mackerras <hidden> Date: 2019-01-31 03:03:15
On Wed, Jan 30, 2019 at 04:54:23PM +0100, Cédric Le Goater wrote:
On 1/30/19 7:20 AM, Paul Mackerras wrote:
quoted
On Tue, Jan 29, 2019 at 02:47:55PM +0100, Cédric Le Goater wrote:
quoted
On 1/29/19 3:45 AM, Paul Mackerras wrote:
quoted
On Mon, Jan 28, 2019 at 07:26:00PM +0100, Cédric Le Goater wrote:
quoted
On 1/28/19 7:13 AM, Paul Mackerras wrote:
quoted
Would we end up with too many VMAs if we just used mmap() to
change the mappings from the software-generated pages to the
hardware-generated interrupt pages?
The sPAPR IRQ number space is 0x8000 wide now. The first 4K are
dedicated to CPU IPIs and the remaining 4K are for devices. We can
Confused. You say the number space has 32768 entries but then imply
there are only 8K entries. Do you mean that the API allows for 15-bit
IRQ numbers but we are only making using of 8192 of them?
extend the last range if needed as these are for MSIs. Dynamic
extensions under KVM should work also.
This to say that we have with 8K x 2 (trigger+EOI) pages. This is a
lot of mmap(), too much. Also the KVM model needs to be compatible
I wasn't suggesting an mmap per IRQ, I meant that the bulk of the
space would be covered by a single mmap, overlaid by subsequent mmaps
where we need to map real device interrupts.
ok. The same fault handler could be used to populate the VMA with the
ESB pages.
But it would mean extra work on the QEMU side, which is not needed
with this patch.
Maybe, but just storing a single vma pointer in our private data is
not a feasible approach. First, you have no control on the lifetime
of the vma and thus this is a use-after-free waiting to happen, and
secondly, there could be multiple vmas that you need to worry about.
I fully agree. That's why I was uncomfortable with the solution. There
are a few other drivers (GPUs if I recall) doing that but it feels wrong.
There is the HMM infrastructure (heterogeneous memory model) which
could possibly be used to do this, but it's very heavyweight for the
problem we have here.
quoted
Userspace could do multiple mmaps, or could do mprotect or similar on
part of an existing mmap, which would split the vma for the mmap into
multiple vmas. You don't get notified about munmap either as far as I
can tell, so the vma is liable to go away.
yes ...
quoted
And it doesn't matter if
QEMU would never do such things; if userspace can provoke a
use-after-free in the kernel using KVM then someone will write a
specially crafted user program to do that.
So we either solve it in userspace, or we have to write and maintain
complex kernel code with deep links into the MM subsystem.
I'd much rather solve it in userspace.
OK, then. I have been reluctant doing so but it seems there are no
other in-kernel solution.
I discussed the problem today with David Gibson and he mentioned that
QEMU does have a lot of freedom in how it assigns the guest hwirq
numbers. So you may be able to avoid the problem by (for example)
never assigning a hwirq to a VFIO device that has previously been used
for an emulated device (or something like that).
quoted
quoted
quoted
quoted
with the QEMU emulated one and it was simpler to have one overall
memory region for the IPI ESBs, one for the END ESBs (if we support
that one day) and one for the TIMA.
quoted
Are the necessary pages for a PCI
passthrough device contiguous in both host real space
They should as they are the PHB4 ESBs.
quoted
and guest real space ?
also. That's how we organized the mapping.
"How we organized the mapping" is a significant design decision that I
haven't seen documented anywhere, and is really needed for
understanding what's going on.
OK. I will add comments on that. See below for some description.
There is nothing fancy, it's simply indexed with the interrupt number,
like for HW, or for the QEMU XIVE emulated model.
quoted
quoted
quoted
If so we'd only need one mmap() for all the device's interrupt
pages.
Ah. So we would want to make a special case for the passthrough
device and have a mmap() and a memory region for its ESBs. Hmm.
Wouldn't that require to hot plug a memory region under the guest ?
No; the way that a memory region works is that userspace can do
whatever disparate mappings it likes within the region on the user
process side, and the corresponding region of guest real address space
follows that automatically.
yes. I suppose this should work also for 'ram device' memory mappings.
So when the passthrough device is added to the guest, we would add a
new 'ram device' memory region for the device interrupt ESB pages
that would overlap with the initial guest ESB pages.
Not knowing the QEMU internals all that well, I don't at all
understand why a new ram device is necesssary.
'ram device' memory regions are of a special type which is used to
directly map into the guest the result of a mmap() in QEMU.
This is how we propagate the XIVE ESB pages from HW (and the TIMA)
to the guest and the Linux kernel. It has other use with VFIO.
quoted
I would see it as a
single virtual area mapping the ESB pages of guest hwirqs that are in
use, and we manage those mappings with mmap and munmap.
Yes I think I understand the idea. I will give a try. I need to find the
right place to do so in QEMU. See other email thread.
quoted
quoted
This is really a different approach.
quoted
quoted
which means that we need to provision an address space/container
region for theses regions. What are the benefits ?
Is clearing the PTEs and repopulating the VMA unsafe ?
Explicitly unmapping parts of the VMA seems like the wrong way to do
it. If you have a device mmap where the device wants to change the
physical page underlying parts of the mapping, there should be a way
for it to do that explicitly (but I don't know off the top of my head
what the interface to do that is).
However, I still haven't seen a clear and concise explanation of what
is being changed, when, and why we need to do that.
Yes. I agree on that. The problem is not very different from what we
have today with the XICS-over-XIVE glue in KVM. Let me try to explain.
The KVM XICS-over-XIVE device and the proposed KVM XIVE native device
implement an IRQ space for the guest using the generic IPI interrupts
of the XIVE IC controller. These interrupts are allocated at the OPAL
level and "mapped" into the guest IRQ number space in the range 0-0x1FFF.
Interrupt management is performed in the XIVE way: using loads and
stores on the addresses of the XIVE IPI interrupt ESB pages.
Both KVM devices share the same internal structure caching information
on the interrupts, among which the xive_irq_data struct containing the
addresses of the IPI ESB pages and an extra one in case of passthrough.
The later contains the addresses of the ESB pages of the underlying HW
controller interrupts, PHB4 in all cases for now.
A guest when running in the XICS legacy interrupt mode lets the KVM
XICS-over-XIVE device "handle" interrupt management, that is to perform
the loads and stores on the addresses of the ESB pages of the guest
interrupts.
However, when running in XIVE native exploitation mode, the KVM XIVE
native device exposes the interrupt ESB pages to the guest and lets
the guest perform directly the loads and stores.
The VMA exposing the ESB pages make use of a custom VM fault handler
which role is to populate the VMA with appropriate pages. When a fault
occurs, the guest IRQ number is deduced from the offset, and the ESB
pages of associated XIVE IPI interrupt are inserted in the VMA (using
the internal structure caching information on the interrupts).
Supporting device passthrough in the guest running in XIVE native
exploitation mode adds some extra refinements because the ESB pages
of a different HW controller (PHB4) need to be exposed to the guest
along with the initial IPI ESB pages of the XIVE IC controller. But
the overall mechanic is the same.
When the device HW irqs are mapped into or unmapped from the guest
IRQ number space, the passthru_irq helpers, kvmppc_xive_set_mapped()
and kvmppc_xive_clr_mapped(), are called to record or clear the
passthrough interrupt information and to perform the switch.
The approach taken by this patch is to clear the ESB pages of the
guest IRQ number being mapped and let the VM fault handler repopulate.
The handler will insert the ESB page corresponding to the HW interrupt
of the device being passed-through or the initial IPI ESB page if the
device is being removed.
From: Paul Mackerras <hidden> Date: 2019-01-31 03:04:51
On Wed, Jan 30, 2019 at 08:01:22AM +0100, Cédric Le Goater wrote:
On 1/30/19 5:29 AM, Paul Mackerras wrote:
quoted
On Mon, Jan 28, 2019 at 06:35:34PM +0100, Cédric Le Goater wrote:
quoted
On 1/22/19 6:05 AM, Paul Mackerras wrote:
quoted
On Mon, Jan 07, 2019 at 07:43:17PM +0100, Cédric Le Goater wrote:
quoted
This is the basic framework for the new KVM device supporting the XIVE
native exploitation mode. The user interface exposes a new capability
and a new KVM device to be used by QEMU.
[snip]
quoted
@@ -1039,7 +1039,10 @@ static int kvmppc_book3s_init(void) #ifdef CONFIG_KVM_XIVE if (xive_enabled()) { kvmppc_xive_init_module();+ kvmppc_xive_native_init_module(); kvm_register_device_ops(&kvm_xive_ops, KVM_DEV_TYPE_XICS);+ kvm_register_device_ops(&kvm_xive_native_ops,+ KVM_DEV_TYPE_XIVE);
I think we want tighter conditions on initializing the xive_native
stuff and creating the xive device class. We could have
xive_enabled() returning true in a guest, and this code will get
called both by PR KVM and HV KVM (and HV KVM no longer implies that we
are running bare metal).
So yes, I gave nested a try with kernel_irqchip=on and the nested hypervisor
(L1) obviously crashes trying to call OPAL. I have tighten the test with :
if (xive_enabled() && !kvmhv_on_pseries()) {
for now.
As this is a problem today in 5.0.x, I will send a patch for it if you think
How do you mean this is a problem today in 5.0? I just tried 5.0-rc1
with kernel_irqchip=on in a nested guest and it works just fine. What
exactly did you test?
L0: Linux 5.0.0-rc3 (+ KVM HV)
L1: QEMU pseries-4.0 (kernel_irqchip=on) - Linux 5.0.0-rc3 (+ KVM HV)
L2: QEMU pseries-4.0 (kernel_irqchip=on) - Linux 5.0.0-rc3
L1 crashes when L2 starts and tries to initialize the KVM IRQ device as
it does an OPAL call and its running under SLOF. See below.
OK, you must have a QEMU that advertises XIVE to the guest (L1). In
that case I can see that L1 would try to do XICS-on-XIVE, which won't
work. We need to fix that. Unfortunately the XICS-on-XICS emulation
won't work as is in L1 either, but I think we can fix that by
disabling the real-mode XICS hcall handling.
I don't understand how L2 can work with kernel_irqchip=on. Could you
please explain ?
If QEMU decides to advertise XIVE to the L2 guest and the L2 guest can
do XIVE, then the only possibility is to use the XIVE software
emulation in QEMU, and if kernel_irqchip=on has been specified
explicitly, maybe QEMU decides to terminate the guest rather than
implicitly turning off kernel_irqchip.
If QEMU decides not to advertise XIVE to the L2 guest, or the L2 guest
can't do XIVE, then we could use the XICS-on-XICS emulation in L1 as
long as either (a) L1 is not using XIVE, or (b) we modify the
XICS-on-XICS code to avoid using any XICS or XIVE access (i.e. just
using calls to generic kernel facilities).
Ultimately, if the spapr xive backend code in the kernel could be
extended to provide all the low-level functions that the XICS-on-XIVE
code needs, then we could do XICS-on-XIVE in a guest.
Paul.
From: Cédric Le Goater <clg@kaod.org> Date: 2019-02-01 17:23:32
On 1/31/19 4:01 AM, Paul Mackerras wrote:
On Wed, Jan 30, 2019 at 08:01:22AM +0100, Cédric Le Goater wrote:
quoted
On 1/30/19 5:29 AM, Paul Mackerras wrote:
quoted
On Mon, Jan 28, 2019 at 06:35:34PM +0100, Cédric Le Goater wrote:
quoted
On 1/22/19 6:05 AM, Paul Mackerras wrote:
quoted
On Mon, Jan 07, 2019 at 07:43:17PM +0100, Cédric Le Goater wrote:
quoted
This is the basic framework for the new KVM device supporting the XIVE
native exploitation mode. The user interface exposes a new capability
and a new KVM device to be used by QEMU.
[snip]
quoted
@@ -1039,7 +1039,10 @@ static int kvmppc_book3s_init(void) #ifdef CONFIG_KVM_XIVE if (xive_enabled()) { kvmppc_xive_init_module();+ kvmppc_xive_native_init_module(); kvm_register_device_ops(&kvm_xive_ops, KVM_DEV_TYPE_XICS);+ kvm_register_device_ops(&kvm_xive_native_ops,+ KVM_DEV_TYPE_XIVE);
I think we want tighter conditions on initializing the xive_native
stuff and creating the xive device class. We could have
xive_enabled() returning true in a guest, and this code will get
called both by PR KVM and HV KVM (and HV KVM no longer implies that we
are running bare metal).
So yes, I gave nested a try with kernel_irqchip=on and the nested hypervisor
(L1) obviously crashes trying to call OPAL. I have tighten the test with :
if (xive_enabled() && !kvmhv_on_pseries()) {
for now.
As this is a problem today in 5.0.x, I will send a patch for it if you think
How do you mean this is a problem today in 5.0? I just tried 5.0-rc1
with kernel_irqchip=on in a nested guest and it works just fine. What
exactly did you test?
L0: Linux 5.0.0-rc3 (+ KVM HV)
L1: QEMU pseries-4.0 (kernel_irqchip=on) - Linux 5.0.0-rc3 (+ KVM HV)
L2: QEMU pseries-4.0 (kernel_irqchip=on) - Linux 5.0.0-rc3
L1 crashes when L2 starts and tries to initialize the KVM IRQ device as
it does an OPAL call and its running under SLOF. See below.
OK, you must have a QEMU that advertises XIVE to the guest (L1).
XIVE is not advertised if QEMU is started with 'ic-mode=xics'
In
that case I can see that L1 would try to do XICS-on-XIVE, which won't
work. We need to fix that. Unfortunately the XICS-on-XICS emulation
won't work as is in L1 either, but I think we can fix that by
disabling the real-mode XICS hcall handling.
I have added some tests on kvm-hv, using kvmhv_on_pseries(), to disable
the KVM XICS-on-XIVE device in a L1 guest running as hypervisor and
to instead register the old KVM XICS device.
If the L1 is started in KVM XICS mode, L2 can now run with KVM XICS.
All seem fine. I booted two guests with disk and network.
But I am still "a bit" confused with what is being done at each
hypervisor level. It's not obvious to follow at all even with traces.
quoted
I don't understand how L2 can work with kernel_irqchip=on. Could you
please explain ?
If QEMU decides to advertise XIVE to the L2 guest and the L2 guest can
do XIVE, then the only possibility is to use the XIVE software
emulation in QEMU, and if kernel_irqchip=on has been specified
explicitly, maybe QEMU decides to terminate the guest rather than
implicitly turning off kernel_irqchip.
we can do that by disabling the KVM XIVE device when under kvmhv_on_pseries().
If QEMU decides not to advertise XIVE to the L2 guest, or the L2 guest
can't do XIVE, then we could use the XICS-on-XICS emulation in L1 as
long as either (a) L1 is not using XIVE, or (b) we modify the
XICS-on-XICS code to avoid using any XICS or XIVE access (i.e. just
using calls to generic kernel facilities).
(a) is what I did above I think
May be we should consider having nested version of the KVM devices
when under kvmhv_on_pseries(). With some sort of backend ops to
modify the relation with the parent hypervisor : PowerNV/Linux or
pseries/Linux.
Ultimately, if the spapr xive backend code in the kernel could be
extended to provide all the low-level functions that the XICS-on-XIVE
code needs, then we could do XICS-on-XIVE in a guest.
What about a XIVE on XIVE ?
Propagating the ESB pages to a nested guest seems feasible if not
already done. The hcalls could be forwarded to the L1 QEMU ? The
problematic part is handling the XIVE VP block.
C.