* [PATCH 0/4] kvm: arm/arm64: vgic: vGICv2 GICC_APRn uaccess support.
@ 2017-07-05 11:23 wanghaibin
2017-07-05 11:23 ` [PATCH 1/4] kvm: arm/arm64: vgic-v2: Add GICH_APR accessors for GICv2 wanghaibin
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: wanghaibin @ 2017-07-05 11:23 UTC (permalink / raw)
To: cdall, marc.zyngier, kvmarm; +Cc: wu.wubin
In the case (GICv2 on GICv3 migration), I did the test on my board as follow:
vm boot => migrate to destination => shutdown at destination => start at destination
=> migrate back to source ... go round and begin again;
I tested many times, but unlucky, there maybe failed by accident when shutdown the vm
at destination. (GICv3 on GICv3 migration, 1000+ times, That's OK).
while failed, we can watch the interrupts in the vm, some vcpus of the vm can not
receive the virt timer interrupt. And, these vcpus will 100% with top tool at host.
vgic_state debug file at destination shows(a active virt timer interrupt) that:
VCPU 0 TYP ID TGT_ID PLAEHC HWID TARGET SRC PRI VCPU_ID
---------------------------------------------------------------
....................
PPI 25 0 000001 0 1 0 160 -1
PPI 26 0 000001 0 1 0 160 -1
PPI 27 0 011111 27 1 0 160 0
I added log to print ICH* registers for VCPU0 at destination:
**HCR:0x1, VMCR:0xf0ec0001,SRE:0x0, ELRSR:0xe**
-----AP0R:0: 0x0------
-----AP0R:1: 0x0------
-----AP0R:2: 0x0------
-----AP0R:3: 0x0------
-----AP1R:0: 0x0------
-----AP1R:1: 0x0------
-----AP1R:2: 0x0------
-----AP1R:3: 0x0------
-----LR:0: 0xa0a0001b0000001b------
-----LR:1: 0x0------
-----LR:2: 0x0------
-----LR:3: 0x0------
and the ICH_AP1R0 value is 0x10000 at source.
At present, QEMU have supproted GICC_APRn put/set interface for emulated GICv2,
and kvm does not support the uaccess interface. This patchset try to support this.
BTW: patch 1 is untested cause I don't have the GICv2 hardware board on hand, sorry.
wanghaibin (4):
kvm: arm/arm64: vgic-v2: Add GICH_APR accessors for GICv2
kvm: arm/arm64: vgic-v3: add ICH_AP[01]Rn accessors for GICv3
kvm: arm/arm64: vgic: Implement the vGICv2 GICC_APRn uaccess
interface.
kvm: arm/arm64: vgic: clean up vGICv3 ICC_APRn sysreg uaccess
arch/arm64/kvm/vgic-sys-reg-v3.c | 25 +++++--------------------
virt/kvm/arm/vgic/vgic-mmio-v2.c | 22 ++++++++++++++++++++--
virt/kvm/arm/vgic/vgic-mmio.c | 28 ++++++++++++++++++++++++++++
virt/kvm/arm/vgic/vgic-v2.c | 21 +++++++++++++++++++++
virt/kvm/arm/vgic/vgic-v3.c | 20 ++++++++++++++++++++
virt/kvm/arm/vgic/vgic.h | 7 +++++++
6 files changed, 101 insertions(+), 22 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/4] kvm: arm/arm64: vgic-v2: Add GICH_APR accessors for GICv2 2017-07-05 11:23 [PATCH 0/4] kvm: arm/arm64: vgic: vGICv2 GICC_APRn uaccess support wanghaibin @ 2017-07-05 11:23 ` wanghaibin 2017-07-06 16:13 ` Marc Zyngier 2017-07-05 11:23 ` [PATCH 2/4] kvm: arm/arm64: vgic-v3: add ICH_AP[01]Rn accessors for GICv3 wanghaibin ` (2 subsequent siblings) 3 siblings, 1 reply; 11+ messages in thread From: wanghaibin @ 2017-07-05 11:23 UTC (permalink / raw) To: cdall, marc.zyngier, kvmarm; +Cc: wu.wubin For GICv2, there are at most 5 priority bits are implemented in GICH_LR<n>.Priority, so we only need to be concerned with GICH_APR0. The other GICH_APRn access can be treated as raz/wi. Attention: This patch is untest! Signed-off-by: wanghaibin <wanghaibin.wang@huawei.com> --- virt/kvm/arm/vgic/vgic-v2.c | 21 +++++++++++++++++++++ virt/kvm/arm/vgic/vgic.h | 2 ++ 2 files changed, 23 insertions(+) diff --git a/virt/kvm/arm/vgic/vgic-v2.c b/virt/kvm/arm/vgic/vgic-v2.c index e4187e5..11d3b73 100644 --- a/virt/kvm/arm/vgic/vgic-v2.c +++ b/virt/kvm/arm/vgic/vgic-v2.c @@ -172,6 +172,27 @@ void vgic_v2_clear_lr(struct kvm_vcpu *vcpu, int lr) vcpu->arch.vgic_cpu.vgic_v2.vgic_lr[lr] = 0; } +void vgic_v2_set_apr(struct kvm_vcpu *vcpu, u32 idx, u32 val) +{ + struct vgic_v2_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v2; + + if (idx == 0) + cpu_if->vgic_apr = val; + else + WARN_ON(val); +} + +u32 vgic_v2_get_apr(struct kvm_vcpu *vcpu, u32 idx) +{ + struct vgic_v2_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v2; + + if (idx == 0) + return cpu_if->vgic_apr; + else + return 0; +} + + void vgic_v2_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcrp) { struct vgic_v2_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v2; diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h index bba7fa2..8791b9a 100644 --- a/virt/kvm/arm/vgic/vgic.h +++ b/virt/kvm/arm/vgic/vgic.h @@ -155,6 +155,8 @@ int vgic_v2_dist_uaccess(struct kvm_vcpu *vcpu, bool is_write, int offset, u32 *val); int vgic_v2_cpuif_uaccess(struct kvm_vcpu *vcpu, bool is_write, int offset, u32 *val); +void vgic_v2_set_apr(struct kvm_vcpu *vcpu, u32 idx, u32 val); +u32 vgic_v2_get_apr(struct kvm_vcpu *vcpu, u32 idx); void vgic_v2_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr); void vgic_v2_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr); void vgic_v2_enable(struct kvm_vcpu *vcpu); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] kvm: arm/arm64: vgic-v2: Add GICH_APR accessors for GICv2 2017-07-05 11:23 ` [PATCH 1/4] kvm: arm/arm64: vgic-v2: Add GICH_APR accessors for GICv2 wanghaibin @ 2017-07-06 16:13 ` Marc Zyngier 2017-07-07 8:37 ` wanghaibin 0 siblings, 1 reply; 11+ messages in thread From: Marc Zyngier @ 2017-07-06 16:13 UTC (permalink / raw) To: wanghaibin, cdall, kvmarm; +Cc: wu.wubin On 05/07/17 12:23, wanghaibin wrote: > For GICv2, there are at most 5 priority bits are implemented in > GICH_LR<n>.Priority, so we only need to be concerned with GICH_APR0. > The other GICH_APRn access can be treated as raz/wi. What is this "other" GICH_APRn? > > Attention: This patch is untest! > > Signed-off-by: wanghaibin <wanghaibin.wang@huawei.com> > --- > virt/kvm/arm/vgic/vgic-v2.c | 21 +++++++++++++++++++++ > virt/kvm/arm/vgic/vgic.h | 2 ++ > 2 files changed, 23 insertions(+) > > diff --git a/virt/kvm/arm/vgic/vgic-v2.c b/virt/kvm/arm/vgic/vgic-v2.c > index e4187e5..11d3b73 100644 > --- a/virt/kvm/arm/vgic/vgic-v2.c > +++ b/virt/kvm/arm/vgic/vgic-v2.c > @@ -172,6 +172,27 @@ void vgic_v2_clear_lr(struct kvm_vcpu *vcpu, int lr) > vcpu->arch.vgic_cpu.vgic_v2.vgic_lr[lr] = 0; > } > > +void vgic_v2_set_apr(struct kvm_vcpu *vcpu, u32 idx, u32 val) > +{ > + struct vgic_v2_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v2; > + > + if (idx == 0) > + cpu_if->vgic_apr = val; > + else > + WARN_ON(val); If treated as WI, why do you WARN here? Also, given that there is only one register for the active priorities, I don't really see the point in having this "idx" parameter. > +} > + > +u32 vgic_v2_get_apr(struct kvm_vcpu *vcpu, u32 idx) > +{ > + struct vgic_v2_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v2; > + > + if (idx == 0) > + return cpu_if->vgic_apr; > + else > + return 0; > +} > + > + Extra whitespace. > void vgic_v2_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcrp) > { > struct vgic_v2_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v2; > diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h > index bba7fa2..8791b9a 100644 > --- a/virt/kvm/arm/vgic/vgic.h > +++ b/virt/kvm/arm/vgic/vgic.h > @@ -155,6 +155,8 @@ int vgic_v2_dist_uaccess(struct kvm_vcpu *vcpu, bool is_write, > int offset, u32 *val); > int vgic_v2_cpuif_uaccess(struct kvm_vcpu *vcpu, bool is_write, > int offset, u32 *val); > +void vgic_v2_set_apr(struct kvm_vcpu *vcpu, u32 idx, u32 val); > +u32 vgic_v2_get_apr(struct kvm_vcpu *vcpu, u32 idx); > void vgic_v2_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr); > void vgic_v2_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr); > void vgic_v2_enable(struct kvm_vcpu *vcpu); > Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] kvm: arm/arm64: vgic-v2: Add GICH_APR accessors for GICv2 2017-07-06 16:13 ` Marc Zyngier @ 2017-07-07 8:37 ` wanghaibin 0 siblings, 0 replies; 11+ messages in thread From: wanghaibin @ 2017-07-07 8:37 UTC (permalink / raw) To: Marc Zyngier; +Cc: cdall, kvmarm, wu.wubin On 2017/7/7 0:13, Marc Zyngier wrote: > On 05/07/17 12:23, wanghaibin wrote: >> For GICv2, there are at most 5 priority bits are implemented in >> GICH_LR<n>.Priority, so we only need to be concerned with GICH_APR0. >> The other GICH_APRn access can be treated as raz/wi. > > What is this "other" GICH_APRn? > >> >> Attention: This patch is untest! >> >> Signed-off-by: wanghaibin <wanghaibin.wang@huawei.com> >> --- >> virt/kvm/arm/vgic/vgic-v2.c | 21 +++++++++++++++++++++ >> virt/kvm/arm/vgic/vgic.h | 2 ++ >> 2 files changed, 23 insertions(+) >> >> diff --git a/virt/kvm/arm/vgic/vgic-v2.c b/virt/kvm/arm/vgic/vgic-v2.c >> index e4187e5..11d3b73 100644 >> --- a/virt/kvm/arm/vgic/vgic-v2.c >> +++ b/virt/kvm/arm/vgic/vgic-v2.c >> @@ -172,6 +172,27 @@ void vgic_v2_clear_lr(struct kvm_vcpu *vcpu, int lr) >> vcpu->arch.vgic_cpu.vgic_v2.vgic_lr[lr] = 0; >> } >> >> +void vgic_v2_set_apr(struct kvm_vcpu *vcpu, u32 idx, u32 val) >> +{ >> + struct vgic_v2_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v2; >> + >> + if (idx == 0) >> + cpu_if->vgic_apr = val; >> + else >> + WARN_ON(val); > > If treated as WI, why do you WARN here? Also, given that there is only > one register for the active priorities, I don't really see the point in > having this "idx" parameter. > About idx parameter, the patch3 will reply. About WARN_ON, GICv2 on GICv2, we just show 5 priority bits currently. If uaccess set apr[1/2/3] with non-zero value, there must be something wrong, here take a warning. Of course, this can be deleted. Thanks. >> +} >> + >> +u32 vgic_v2_get_apr(struct kvm_vcpu *vcpu, u32 idx) >> +{ >> + struct vgic_v2_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v2; >> + >> + if (idx == 0) >> + return cpu_if->vgic_apr; >> + else >> + return 0; >> +} >> + >> + > > Extra whitespace. will fix. Thanks. > >> void vgic_v2_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcrp) >> { >> struct vgic_v2_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v2; >> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h >> index bba7fa2..8791b9a 100644 >> --- a/virt/kvm/arm/vgic/vgic.h >> +++ b/virt/kvm/arm/vgic/vgic.h >> @@ -155,6 +155,8 @@ int vgic_v2_dist_uaccess(struct kvm_vcpu *vcpu, bool is_write, >> int offset, u32 *val); >> int vgic_v2_cpuif_uaccess(struct kvm_vcpu *vcpu, bool is_write, >> int offset, u32 *val); >> +void vgic_v2_set_apr(struct kvm_vcpu *vcpu, u32 idx, u32 val); >> +u32 vgic_v2_get_apr(struct kvm_vcpu *vcpu, u32 idx); >> void vgic_v2_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr); >> void vgic_v2_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr); >> void vgic_v2_enable(struct kvm_vcpu *vcpu); >> > > Thanks, > > M. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/4] kvm: arm/arm64: vgic-v3: add ICH_AP[01]Rn accessors for GICv3 2017-07-05 11:23 [PATCH 0/4] kvm: arm/arm64: vgic: vGICv2 GICC_APRn uaccess support wanghaibin 2017-07-05 11:23 ` [PATCH 1/4] kvm: arm/arm64: vgic-v2: Add GICH_APR accessors for GICv2 wanghaibin @ 2017-07-05 11:23 ` wanghaibin 2017-07-06 16:18 ` Marc Zyngier 2017-07-05 11:23 ` [PATCH 3/4] kvm: arm/arm64: vgic: Implement the vGICv2 GICC_APRn uaccess interface wanghaibin 2017-07-05 11:23 ` [PATCH 4/4] kvm: arm/arm64: vgic: clean up vGICv3 ICC_APRn sysreg uaccess wanghaibin 3 siblings, 1 reply; 11+ messages in thread From: wanghaibin @ 2017-07-05 11:23 UTC (permalink / raw) To: cdall, marc.zyngier, kvmarm; +Cc: wu.wubin Compared to GICv2 hardware, GICv3 support ICH_AP[01]Rn for Group [01] interrupts. Signed-off-by: wanghaibin <wanghaibin.wang@huawei.com> --- virt/kvm/arm/vgic/vgic-v3.c | 20 ++++++++++++++++++++ virt/kvm/arm/vgic/vgic.h | 2 ++ 2 files changed, 22 insertions(+) diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c index 030248e..361da71 100644 --- a/virt/kvm/arm/vgic/vgic-v3.c +++ b/virt/kvm/arm/vgic/vgic-v3.c @@ -156,6 +156,26 @@ void vgic_v3_clear_lr(struct kvm_vcpu *vcpu, int lr) vcpu->arch.vgic_cpu.vgic_v3.vgic_lr[lr] = 0; } +void vgic_v3_set_apr(struct kvm_vcpu *vcpu, u8 apr, u32 idx, u32 val) +{ + struct vgic_v3_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v3; + + if (apr) + cpu_if->vgic_ap1r[idx] = val; + else + cpu_if->vgic_ap0r[idx] = val; +} + +u32 vgic_v3_get_apr(struct kvm_vcpu *vcpu, u8 apr, u32 idx) +{ + struct vgic_v3_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v3; + + if (apr) + return cpu_if->vgic_ap1r[idx]; + else + return cpu_if->vgic_ap0r[idx]; +} + void vgic_v3_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcrp) { struct vgic_v3_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v3; diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h index 8791b9a..91d66ec 100644 --- a/virt/kvm/arm/vgic/vgic.h +++ b/virt/kvm/arm/vgic/vgic.h @@ -181,6 +181,8 @@ static inline void vgic_get_irq_kref(struct vgic_irq *irq) void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr); void vgic_v3_clear_lr(struct kvm_vcpu *vcpu, int lr); void vgic_v3_set_underflow(struct kvm_vcpu *vcpu); +void vgic_v3_set_apr(struct kvm_vcpu *vcpu, u8 apr, u32 idx, u32 val); +u32 vgic_v3_get_apr(struct kvm_vcpu *vcpu, u8 apr, u32 idx); void vgic_v3_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr); void vgic_v3_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr); void vgic_v3_enable(struct kvm_vcpu *vcpu); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/4] kvm: arm/arm64: vgic-v3: add ICH_AP[01]Rn accessors for GICv3 2017-07-05 11:23 ` [PATCH 2/4] kvm: arm/arm64: vgic-v3: add ICH_AP[01]Rn accessors for GICv3 wanghaibin @ 2017-07-06 16:18 ` Marc Zyngier 2017-07-07 9:22 ` wanghaibin 0 siblings, 1 reply; 11+ messages in thread From: Marc Zyngier @ 2017-07-06 16:18 UTC (permalink / raw) To: wanghaibin, cdall, kvmarm; +Cc: wu.wubin On 05/07/17 12:23, wanghaibin wrote: > Compared to GICv2 hardware, GICv3 support ICH_AP[01]Rn for Group [01] > interrupts. This doesn't describe what the patch is trying to achieve. > > Signed-off-by: wanghaibin <wanghaibin.wang@huawei.com> > --- > virt/kvm/arm/vgic/vgic-v3.c | 20 ++++++++++++++++++++ > virt/kvm/arm/vgic/vgic.h | 2 ++ > 2 files changed, 22 insertions(+) > > diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c > index 030248e..361da71 100644 > --- a/virt/kvm/arm/vgic/vgic-v3.c > +++ b/virt/kvm/arm/vgic/vgic-v3.c > @@ -156,6 +156,26 @@ void vgic_v3_clear_lr(struct kvm_vcpu *vcpu, int lr) > vcpu->arch.vgic_cpu.vgic_v3.vgic_lr[lr] = 0; > } > > +void vgic_v3_set_apr(struct kvm_vcpu *vcpu, u8 apr, u32 idx, u32 val) The "apr" parameter is actually the group number, and should be exposed as such. > +{ > + struct vgic_v3_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v3; > + > + if (apr) > + cpu_if->vgic_ap1r[idx] = val; > + else > + cpu_if->vgic_ap0r[idx] = val; How do you guarantee that you're not accessing outside of this GIC's preemption capability? > +} > + > +u32 vgic_v3_get_apr(struct kvm_vcpu *vcpu, u8 apr, u32 idx) > +{ > + struct vgic_v3_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v3; > + > + if (apr) > + return cpu_if->vgic_ap1r[idx]; > + else > + return cpu_if->vgic_ap0r[idx]; > +} > + > void vgic_v3_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcrp) > { > struct vgic_v3_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v3; > diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h > index 8791b9a..91d66ec 100644 > --- a/virt/kvm/arm/vgic/vgic.h > +++ b/virt/kvm/arm/vgic/vgic.h > @@ -181,6 +181,8 @@ static inline void vgic_get_irq_kref(struct vgic_irq *irq) > void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr); > void vgic_v3_clear_lr(struct kvm_vcpu *vcpu, int lr); > void vgic_v3_set_underflow(struct kvm_vcpu *vcpu); > +void vgic_v3_set_apr(struct kvm_vcpu *vcpu, u8 apr, u32 idx, u32 val); > +u32 vgic_v3_get_apr(struct kvm_vcpu *vcpu, u8 apr, u32 idx); > void vgic_v3_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr); > void vgic_v3_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr); > void vgic_v3_enable(struct kvm_vcpu *vcpu); > Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/4] kvm: arm/arm64: vgic-v3: add ICH_AP[01]Rn accessors for GICv3 2017-07-06 16:18 ` Marc Zyngier @ 2017-07-07 9:22 ` wanghaibin 0 siblings, 0 replies; 11+ messages in thread From: wanghaibin @ 2017-07-07 9:22 UTC (permalink / raw) To: Marc Zyngier; +Cc: cdall, kvmarm, wu.wubin On 2017/7/7 0:18, Marc Zyngier wrote: > On 05/07/17 12:23, wanghaibin wrote: >> Compared to GICv2 hardware, GICv3 support ICH_AP[01]Rn for Group [01] >> interrupts. > > This doesn't describe what the patch is trying to achieve. Will fix. > >> >> Signed-off-by: wanghaibin <wanghaibin.wang@huawei.com> >> --- >> virt/kvm/arm/vgic/vgic-v3.c | 20 ++++++++++++++++++++ >> virt/kvm/arm/vgic/vgic.h | 2 ++ >> 2 files changed, 22 insertions(+) >> >> diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c >> index 030248e..361da71 100644 >> --- a/virt/kvm/arm/vgic/vgic-v3.c >> +++ b/virt/kvm/arm/vgic/vgic-v3.c >> @@ -156,6 +156,26 @@ void vgic_v3_clear_lr(struct kvm_vcpu *vcpu, int lr) >> vcpu->arch.vgic_cpu.vgic_v3.vgic_lr[lr] = 0; >> } >> >> +void vgic_v3_set_apr(struct kvm_vcpu *vcpu, u8 apr, u32 idx, u32 val) > > The "apr" parameter is actually the group number, and should be exposed > as such. > Will fix. >> +{ >> + struct vgic_v3_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v3; >> + >> + if (apr) >> + cpu_if->vgic_ap1r[idx] = val; >> + else >> + cpu_if->vgic_ap0r[idx] = val; > > How do you guarantee that you're not accessing outside of this GIC's > preemption capability? > Only GICC_APRn mmio uaccess for vGICv2 and ICC_APRn uaccess sysreg uaccess will calculate and pass the idx parameter to here, and these action can guarantee the idx only can be set 0,1,2,3. Thanks. >> +} >> + >> +u32 vgic_v3_get_apr(struct kvm_vcpu *vcpu, u8 apr, u32 idx) >> +{ >> + struct vgic_v3_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v3; >> + >> + if (apr) >> + return cpu_if->vgic_ap1r[idx]; >> + else >> + return cpu_if->vgic_ap0r[idx]; >> +} >> + >> void vgic_v3_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcrp) >> { >> struct vgic_v3_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v3; >> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h >> index 8791b9a..91d66ec 100644 >> --- a/virt/kvm/arm/vgic/vgic.h >> +++ b/virt/kvm/arm/vgic/vgic.h >> @@ -181,6 +181,8 @@ static inline void vgic_get_irq_kref(struct vgic_irq *irq) >> void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr); >> void vgic_v3_clear_lr(struct kvm_vcpu *vcpu, int lr); >> void vgic_v3_set_underflow(struct kvm_vcpu *vcpu); >> +void vgic_v3_set_apr(struct kvm_vcpu *vcpu, u8 apr, u32 idx, u32 val); >> +u32 vgic_v3_get_apr(struct kvm_vcpu *vcpu, u8 apr, u32 idx); >> void vgic_v3_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr); >> void vgic_v3_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr); >> void vgic_v3_enable(struct kvm_vcpu *vcpu); >> > > Thanks, > > M. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/4] kvm: arm/arm64: vgic: Implement the vGICv2 GICC_APRn uaccess interface. 2017-07-05 11:23 [PATCH 0/4] kvm: arm/arm64: vgic: vGICv2 GICC_APRn uaccess support wanghaibin 2017-07-05 11:23 ` [PATCH 1/4] kvm: arm/arm64: vgic-v2: Add GICH_APR accessors for GICv2 wanghaibin 2017-07-05 11:23 ` [PATCH 2/4] kvm: arm/arm64: vgic-v3: add ICH_AP[01]Rn accessors for GICv3 wanghaibin @ 2017-07-05 11:23 ` wanghaibin 2017-07-06 16:53 ` Marc Zyngier 2017-07-05 11:23 ` [PATCH 4/4] kvm: arm/arm64: vgic: clean up vGICv3 ICC_APRn sysreg uaccess wanghaibin 3 siblings, 1 reply; 11+ messages in thread From: wanghaibin @ 2017-07-05 11:23 UTC (permalink / raw) To: cdall, marc.zyngier, kvmarm; +Cc: wu.wubin Access GICV_APRn hardware register, SPEC says: When System register access is disabled for EL2, these registers access GICH_APR<n>, and all active priorities for virtual machines are held in GICH_APR<n> regardless of interrupt group. When System register access is enabled for EL2, these registers access ICH_AP1R<n>_EL2, and all active priorities for virtual machines are held in ICH_AP1R<n>_EL2 regardless of interrupt group. Accordingly, the following two cases are implemented: (1) GICv2 on GICv2, uaccess GICC_APRn info from GICH_APRn, and, (2) GICv2 on GICv3, uaccess GICC_APRn info from ICH_AP1Rn. Signed-off-by: wanghaibin <wanghaibin.wang@huawei.com> --- virt/kvm/arm/vgic/vgic-mmio-v2.c | 22 ++++++++++++++++++++-- virt/kvm/arm/vgic/vgic-mmio.c | 28 ++++++++++++++++++++++++++++ virt/kvm/arm/vgic/vgic.h | 3 +++ 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/virt/kvm/arm/vgic/vgic-mmio-v2.c b/virt/kvm/arm/vgic/vgic-mmio-v2.c index 63e0bbd..176b93f 100644 --- a/virt/kvm/arm/vgic/vgic-mmio-v2.c +++ b/virt/kvm/arm/vgic/vgic-mmio-v2.c @@ -303,6 +303,23 @@ static void vgic_mmio_write_vcpuif(struct kvm_vcpu *vcpu, vgic_set_vmcr(vcpu, &vmcr); } +static unsigned long vgic_v2_uaccess_read_activeprio(struct kvm_vcpu *vcpu, + gpa_t addr, unsigned int len) +{ + u32 idx = (addr & 0x0c) / sizeof(u32); + + return vgic_get_apr(vcpu, 0, idx); +} + +static void vgic_v2_uaccess_write_activeprio(struct kvm_vcpu *vcpu, + gpa_t addr, unsigned int len, + unsigned long val) +{ + u32 idx = (addr & 0x0c) / sizeof(u32); + + return vgic_set_apr(vcpu, 0, idx, val); +} + static const struct vgic_register_region vgic_v2_dist_registers[] = { REGISTER_DESC_WITH_LENGTH(GIC_DIST_CTRL, vgic_mmio_read_v2_misc, vgic_mmio_write_v2_misc, 12, @@ -361,8 +378,9 @@ static void vgic_mmio_write_vcpuif(struct kvm_vcpu *vcpu, REGISTER_DESC_WITH_LENGTH(GIC_CPU_ALIAS_BINPOINT, vgic_mmio_read_vcpuif, vgic_mmio_write_vcpuif, 4, VGIC_ACCESS_32bit), - REGISTER_DESC_WITH_LENGTH(GIC_CPU_ACTIVEPRIO, - vgic_mmio_read_raz, vgic_mmio_write_wi, 16, + REGISTER_DESC_WITH_LENGTH_UACCESS(GIC_CPU_ACTIVEPRIO, + vgic_mmio_read_raz, vgic_mmio_write_wi, + vgic_v2_uaccess_read_activeprio, vgic_v2_uaccess_write_activeprio, 16, VGIC_ACCESS_32bit), REGISTER_DESC_WITH_LENGTH(GIC_CPU_IDENT, vgic_mmio_read_vcpuif, vgic_mmio_write_vcpuif, 4, diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c index 1c17b2a..42fe00d 100644 --- a/virt/kvm/arm/vgic/vgic-mmio.c +++ b/virt/kvm/arm/vgic/vgic-mmio.c @@ -454,6 +454,34 @@ static int match_region(const void *key, const void *elt) sizeof(regions[0]), match_region); } +void vgic_set_apr(struct kvm_vcpu *vcpu, u8 apr, u32 idx, u32 val) +{ + u32 vgic_model = vcpu->kvm->arch.vgic.vgic_model; + + if (kvm_vgic_global_state.type == VGIC_V2) + vgic_v2_set_apr(vcpu, idx, val); + else { + if (vgic_model == KVM_DEV_TYPE_ARM_VGIC_V2) + vgic_v3_set_apr(vcpu, 1, idx, val); + else + vgic_v3_set_apr(vcpu, apr, idx, val); + } +} + +u32 vgic_get_apr(struct kvm_vcpu *vcpu, u8 apr, u32 idx) +{ + u32 vgic_model = vcpu->kvm->arch.vgic.vgic_model; + + if (kvm_vgic_global_state.type == VGIC_V2) + return vgic_v2_get_apr(vcpu, idx); + else { + if (vgic_model == KVM_DEV_TYPE_ARM_VGIC_V2) + return vgic_v3_get_apr(vcpu, 1, idx); + else + return vgic_v3_get_apr(vcpu, apr, idx); + } +} + void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr) { if (kvm_vgic_global_state.type == VGIC_V2) diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h index 91d66ec..3cdc448 100644 --- a/virt/kvm/arm/vgic/vgic.h +++ b/virt/kvm/arm/vgic/vgic.h @@ -213,6 +213,9 @@ int vgic_v3_has_cpu_sysregs_attr(struct kvm_vcpu *vcpu, bool is_write, u64 id, int vgic_v3_line_level_info_uaccess(struct kvm_vcpu *vcpu, bool is_write, u32 intid, u64 *val); int kvm_register_vgic_device(unsigned long type); + +void vgic_set_apr(struct kvm_vcpu *vcpu, u8 apr, u32 idx, u32 val); +u32 vgic_get_apr(struct kvm_vcpu *vcpu, u8 apr, u32 idx); void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr); void vgic_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr); int vgic_lazy_init(struct kvm *kvm); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] kvm: arm/arm64: vgic: Implement the vGICv2 GICC_APRn uaccess interface. 2017-07-05 11:23 ` [PATCH 3/4] kvm: arm/arm64: vgic: Implement the vGICv2 GICC_APRn uaccess interface wanghaibin @ 2017-07-06 16:53 ` Marc Zyngier 2017-07-07 8:28 ` wanghaibin 0 siblings, 1 reply; 11+ messages in thread From: Marc Zyngier @ 2017-07-06 16:53 UTC (permalink / raw) To: wanghaibin, cdall, kvmarm; +Cc: wu.wubin On 05/07/17 12:23, wanghaibin wrote: > Access GICV_APRn hardware register, SPEC says: > > When System register access is disabled for EL2, these registers access GICH_APR<n>, > and all active priorities for virtual machines are held in GICH_APR<n> regardless of > interrupt group. > When System register access is enabled for EL2, these registers access ICH_AP1R<n>_EL2, > and all active priorities for virtual machines are held in ICH_AP1R<n>_EL2 regardless > of interrupt group. > > Accordingly, the following two cases are implemented: > (1) GICv2 on GICv2, uaccess GICC_APRn info from GICH_APRn, and, > (2) GICv2 on GICv3, uaccess GICC_APRn info from ICH_AP1Rn. > > Signed-off-by: wanghaibin <wanghaibin.wang@huawei.com> > --- > virt/kvm/arm/vgic/vgic-mmio-v2.c | 22 ++++++++++++++++++++-- > virt/kvm/arm/vgic/vgic-mmio.c | 28 ++++++++++++++++++++++++++++ > virt/kvm/arm/vgic/vgic.h | 3 +++ > 3 files changed, 51 insertions(+), 2 deletions(-) > > diff --git a/virt/kvm/arm/vgic/vgic-mmio-v2.c b/virt/kvm/arm/vgic/vgic-mmio-v2.c > index 63e0bbd..176b93f 100644 > --- a/virt/kvm/arm/vgic/vgic-mmio-v2.c > +++ b/virt/kvm/arm/vgic/vgic-mmio-v2.c > @@ -303,6 +303,23 @@ static void vgic_mmio_write_vcpuif(struct kvm_vcpu *vcpu, > vgic_set_vmcr(vcpu, &vmcr); > } > > +static unsigned long vgic_v2_uaccess_read_activeprio(struct kvm_vcpu *vcpu, > + gpa_t addr, unsigned int len) > +{ > + u32 idx = (addr & 0x0c) / sizeof(u32); > + > + return vgic_get_apr(vcpu, 0, idx); > +} > + > +static void vgic_v2_uaccess_write_activeprio(struct kvm_vcpu *vcpu, > + gpa_t addr, unsigned int len, > + unsigned long val) > +{ > + u32 idx = (addr & 0x0c) / sizeof(u32); > + > + return vgic_set_apr(vcpu, 0, idx, val); > +} > + > static const struct vgic_register_region vgic_v2_dist_registers[] = { > REGISTER_DESC_WITH_LENGTH(GIC_DIST_CTRL, > vgic_mmio_read_v2_misc, vgic_mmio_write_v2_misc, 12, > @@ -361,8 +378,9 @@ static void vgic_mmio_write_vcpuif(struct kvm_vcpu *vcpu, > REGISTER_DESC_WITH_LENGTH(GIC_CPU_ALIAS_BINPOINT, > vgic_mmio_read_vcpuif, vgic_mmio_write_vcpuif, 4, > VGIC_ACCESS_32bit), > - REGISTER_DESC_WITH_LENGTH(GIC_CPU_ACTIVEPRIO, > - vgic_mmio_read_raz, vgic_mmio_write_wi, 16, > + REGISTER_DESC_WITH_LENGTH_UACCESS(GIC_CPU_ACTIVEPRIO, > + vgic_mmio_read_raz, vgic_mmio_write_wi, > + vgic_v2_uaccess_read_activeprio, vgic_v2_uaccess_write_activeprio, 16, This 16 feels wrong. I now see why you had this index in the first patch. I think it begs the questions of what we're emulating. Are we showing a virtual GICv2 with only 5 bits of priority? Or something else? > VGIC_ACCESS_32bit), > REGISTER_DESC_WITH_LENGTH(GIC_CPU_IDENT, > vgic_mmio_read_vcpuif, vgic_mmio_write_vcpuif, 4, > diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c > index 1c17b2a..42fe00d 100644 > --- a/virt/kvm/arm/vgic/vgic-mmio.c > +++ b/virt/kvm/arm/vgic/vgic-mmio.c > @@ -454,6 +454,34 @@ static int match_region(const void *key, const void *elt) > sizeof(regions[0]), match_region); > } > > +void vgic_set_apr(struct kvm_vcpu *vcpu, u8 apr, u32 idx, u32 val) Same remark as before about "apr". > +{ > + u32 vgic_model = vcpu->kvm->arch.vgic.vgic_model; > + > + if (kvm_vgic_global_state.type == VGIC_V2) > + vgic_v2_set_apr(vcpu, idx, val); > + else { > + if (vgic_model == KVM_DEV_TYPE_ARM_VGIC_V2) > + vgic_v3_set_apr(vcpu, 1, idx, val); > + else > + vgic_v3_set_apr(vcpu, apr, idx, val); > + } > +} > + > +u32 vgic_get_apr(struct kvm_vcpu *vcpu, u8 apr, u32 idx) > +{ > + u32 vgic_model = vcpu->kvm->arch.vgic.vgic_model; > + > + if (kvm_vgic_global_state.type == VGIC_V2) > + return vgic_v2_get_apr(vcpu, idx); > + else { > + if (vgic_model == KVM_DEV_TYPE_ARM_VGIC_V2) > + return vgic_v3_get_apr(vcpu, 1, idx); > + else > + return vgic_v3_get_apr(vcpu, apr, idx); > + } > +} > + > void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr) > { > if (kvm_vgic_global_state.type == VGIC_V2) > diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h > index 91d66ec..3cdc448 100644 > --- a/virt/kvm/arm/vgic/vgic.h > +++ b/virt/kvm/arm/vgic/vgic.h > @@ -213,6 +213,9 @@ int vgic_v3_has_cpu_sysregs_attr(struct kvm_vcpu *vcpu, bool is_write, u64 id, > int vgic_v3_line_level_info_uaccess(struct kvm_vcpu *vcpu, bool is_write, > u32 intid, u64 *val); > int kvm_register_vgic_device(unsigned long type); > + > +void vgic_set_apr(struct kvm_vcpu *vcpu, u8 apr, u32 idx, u32 val); > +u32 vgic_get_apr(struct kvm_vcpu *vcpu, u8 apr, u32 idx); > void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr); > void vgic_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr); > int vgic_lazy_init(struct kvm *kvm); > Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] kvm: arm/arm64: vgic: Implement the vGICv2 GICC_APRn uaccess interface. 2017-07-06 16:53 ` Marc Zyngier @ 2017-07-07 8:28 ` wanghaibin 0 siblings, 0 replies; 11+ messages in thread From: wanghaibin @ 2017-07-07 8:28 UTC (permalink / raw) To: Marc Zyngier; +Cc: cdall, kvmarm, wu.wubin On 2017/7/7 0:53, Marc Zyngier wrote: > On 05/07/17 12:23, wanghaibin wrote: >> Access GICV_APRn hardware register, SPEC says: >> >> When System register access is disabled for EL2, these registers access GICH_APR<n>, >> and all active priorities for virtual machines are held in GICH_APR<n> regardless of >> interrupt group. >> When System register access is enabled for EL2, these registers access ICH_AP1R<n>_EL2, >> and all active priorities for virtual machines are held in ICH_AP1R<n>_EL2 regardless >> of interrupt group. >> >> Accordingly, the following two cases are implemented: >> (1) GICv2 on GICv2, uaccess GICC_APRn info from GICH_APRn, and, >> (2) GICv2 on GICv3, uaccess GICC_APRn info from ICH_AP1Rn. >> >> Signed-off-by: wanghaibin <wanghaibin.wang@huawei.com> >> --- >> virt/kvm/arm/vgic/vgic-mmio-v2.c | 22 ++++++++++++++++++++-- >> virt/kvm/arm/vgic/vgic-mmio.c | 28 ++++++++++++++++++++++++++++ >> virt/kvm/arm/vgic/vgic.h | 3 +++ >> 3 files changed, 51 insertions(+), 2 deletions(-) >> >> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v2.c b/virt/kvm/arm/vgic/vgic-mmio-v2.c >> index 63e0bbd..176b93f 100644 >> --- a/virt/kvm/arm/vgic/vgic-mmio-v2.c >> +++ b/virt/kvm/arm/vgic/vgic-mmio-v2.c >> @@ -303,6 +303,23 @@ static void vgic_mmio_write_vcpuif(struct kvm_vcpu *vcpu, >> vgic_set_vmcr(vcpu, &vmcr); >> } >> >> +static unsigned long vgic_v2_uaccess_read_activeprio(struct kvm_vcpu *vcpu, >> + gpa_t addr, unsigned int len) >> +{ >> + u32 idx = (addr & 0x0c) / sizeof(u32); >> + >> + return vgic_get_apr(vcpu, 0, idx); >> +} >> + >> +static void vgic_v2_uaccess_write_activeprio(struct kvm_vcpu *vcpu, >> + gpa_t addr, unsigned int len, >> + unsigned long val) >> +{ >> + u32 idx = (addr & 0x0c) / sizeof(u32); >> + >> + return vgic_set_apr(vcpu, 0, idx, val); >> +} >> + >> static const struct vgic_register_region vgic_v2_dist_registers[] = { >> REGISTER_DESC_WITH_LENGTH(GIC_DIST_CTRL, >> vgic_mmio_read_v2_misc, vgic_mmio_write_v2_misc, 12, >> @@ -361,8 +378,9 @@ static void vgic_mmio_write_vcpuif(struct kvm_vcpu *vcpu, >> REGISTER_DESC_WITH_LENGTH(GIC_CPU_ALIAS_BINPOINT, >> vgic_mmio_read_vcpuif, vgic_mmio_write_vcpuif, 4, >> VGIC_ACCESS_32bit), >> - REGISTER_DESC_WITH_LENGTH(GIC_CPU_ACTIVEPRIO, >> - vgic_mmio_read_raz, vgic_mmio_write_wi, 16, >> + REGISTER_DESC_WITH_LENGTH_UACCESS(GIC_CPU_ACTIVEPRIO, >> + vgic_mmio_read_raz, vgic_mmio_write_wi, >> + vgic_v2_uaccess_read_activeprio, vgic_v2_uaccess_write_activeprio, 16, > > This 16 feels wrong. I now see why you had this index in the first > patch. I think it begs the questions of what we're emulating. Are we > showing a virtual GICv2 with only 5 bits of priority? Or something else? Hi, Marc: At present, Yes, I think it's only 5 bits of priority to show for GICv2. But， the QEMU gicc apr put/get access code just like: /* s->apr[n][cpu] -> GICC_APRn */ for (i = 0; i < 4; i++) { reg = s->apr[i][cpu]; kvm_gicc_access(s, 0xd0 + i * 4, cpu, ®, true); } It get/put all gicc apr registers (the spec implement GICC_APR<n> (n = 0 - 3) to provides information about interrupt active priorities), So I think the 16 is correct. BTW: I notice that we show the 5 priority bits by VGIC_PRI_BITS to filter emulate write GICD_IPRIORITYRn bits （vgic_mmio_write_priority） both emulate GICv2 and emulate GICv3. Should we cancel this limit when (GICv2 on GICv3) and (GICv3 on GICv3) ? I think we can show the bits which the hardware ICH_VTR_EL2.PRIbits support when the hardware is GICv3 to avoid priority bits lost. Thanks. > >> VGIC_ACCESS_32bit), >> REGISTER_DESC_WITH_LENGTH(GIC_CPU_IDENT, >> vgic_mmio_read_vcpuif, vgic_mmio_write_vcpuif, 4, >> diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c >> index 1c17b2a..42fe00d 100644 >> --- a/virt/kvm/arm/vgic/vgic-mmio.c >> +++ b/virt/kvm/arm/vgic/vgic-mmio.c >> @@ -454,6 +454,34 @@ static int match_region(const void *key, const void *elt) >> sizeof(regions[0]), match_region); >> } >> >> +void vgic_set_apr(struct kvm_vcpu *vcpu, u8 apr, u32 idx, u32 val) > > Same remark as before about "apr". > >> +{ >> + u32 vgic_model = vcpu->kvm->arch.vgic.vgic_model; >> + >> + if (kvm_vgic_global_state.type == VGIC_V2) >> + vgic_v2_set_apr(vcpu, idx, val); >> + else { >> + if (vgic_model == KVM_DEV_TYPE_ARM_VGIC_V2) >> + vgic_v3_set_apr(vcpu, 1, idx, val); >> + else >> + vgic_v3_set_apr(vcpu, apr, idx, val); >> + } >> +} >> + >> +u32 vgic_get_apr(struct kvm_vcpu *vcpu, u8 apr, u32 idx) >> +{ >> + u32 vgic_model = vcpu->kvm->arch.vgic.vgic_model; >> + >> + if (kvm_vgic_global_state.type == VGIC_V2) >> + return vgic_v2_get_apr(vcpu, idx); >> + else { >> + if (vgic_model == KVM_DEV_TYPE_ARM_VGIC_V2) >> + return vgic_v3_get_apr(vcpu, 1, idx); >> + else >> + return vgic_v3_get_apr(vcpu, apr, idx); >> + } >> +} >> + >> void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr) >> { >> if (kvm_vgic_global_state.type == VGIC_V2) >> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h >> index 91d66ec..3cdc448 100644 >> --- a/virt/kvm/arm/vgic/vgic.h >> +++ b/virt/kvm/arm/vgic/vgic.h >> @@ -213,6 +213,9 @@ int vgic_v3_has_cpu_sysregs_attr(struct kvm_vcpu *vcpu, bool is_write, u64 id, >> int vgic_v3_line_level_info_uaccess(struct kvm_vcpu *vcpu, bool is_write, >> u32 intid, u64 *val); >> int kvm_register_vgic_device(unsigned long type); >> + >> +void vgic_set_apr(struct kvm_vcpu *vcpu, u8 apr, u32 idx, u32 val); >> +u32 vgic_get_apr(struct kvm_vcpu *vcpu, u8 apr, u32 idx); >> void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr); >> void vgic_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr); >> int vgic_lazy_init(struct kvm *kvm); >> > > Thanks, > > M. _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/4] kvm: arm/arm64: vgic: clean up vGICv3 ICC_APRn sysreg uaccess 2017-07-05 11:23 [PATCH 0/4] kvm: arm/arm64: vgic: vGICv2 GICC_APRn uaccess support wanghaibin ` (2 preceding siblings ...) 2017-07-05 11:23 ` [PATCH 3/4] kvm: arm/arm64: vgic: Implement the vGICv2 GICC_APRn uaccess interface wanghaibin @ 2017-07-05 11:23 ` wanghaibin 3 siblings, 0 replies; 11+ messages in thread From: wanghaibin @ 2017-07-05 11:23 UTC (permalink / raw) To: cdall, marc.zyngier, kvmarm; +Cc: wu.wubin The vGICv3 ICC_APRn sysreg uaccess should call the APR access common interface. Signed-off-by: wanghaibin <wanghaibin.wang@huawei.com> --- arch/arm64/kvm/vgic-sys-reg-v3.c | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/arch/arm64/kvm/vgic-sys-reg-v3.c b/arch/arm64/kvm/vgic-sys-reg-v3.c index 6260b69..0f7fbcd 100644 --- a/arch/arm64/kvm/vgic-sys-reg-v3.c +++ b/arch/arm64/kvm/vgic-sys-reg-v3.c @@ -188,23 +188,6 @@ static bool access_gic_grpen1(struct kvm_vcpu *vcpu, struct sys_reg_params *p, return true; } -static void vgic_v3_access_apr_reg(struct kvm_vcpu *vcpu, - struct sys_reg_params *p, u8 apr, u8 idx) -{ - struct vgic_v3_cpu_if *vgicv3 = &vcpu->arch.vgic_cpu.vgic_v3; - uint32_t *ap_reg; - - if (apr) - ap_reg = &vgicv3->vgic_ap1r[idx]; - else - ap_reg = &vgicv3->vgic_ap0r[idx]; - - if (p->is_write) - *ap_reg = p->regval; - else - p->regval = *ap_reg; -} - static bool access_gic_aprn(struct kvm_vcpu *vcpu, struct sys_reg_params *p, const struct sys_reg_desc *r, u8 apr) { @@ -218,19 +201,21 @@ static bool access_gic_aprn(struct kvm_vcpu *vcpu, struct sys_reg_params *p, */ switch (vgic_v3_cpu->num_pri_bits) { case 7: - vgic_v3_access_apr_reg(vcpu, p, apr, idx); break; case 6: if (idx > 1) goto err; - vgic_v3_access_apr_reg(vcpu, p, apr, idx); break; default: if (idx > 0) goto err; - vgic_v3_access_apr_reg(vcpu, p, apr, idx); } + if (p->is_write) + vgic_set_apr(vcpu, apr, idx, p->regval); + else + p->regval = vgic_get_apr(vcpu, apr, idx); + return true; err: if (!p->is_write) -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-07-07 9:22 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-07-05 11:23 [PATCH 0/4] kvm: arm/arm64: vgic: vGICv2 GICC_APRn uaccess support wanghaibin 2017-07-05 11:23 ` [PATCH 1/4] kvm: arm/arm64: vgic-v2: Add GICH_APR accessors for GICv2 wanghaibin 2017-07-06 16:13 ` Marc Zyngier 2017-07-07 8:37 ` wanghaibin 2017-07-05 11:23 ` [PATCH 2/4] kvm: arm/arm64: vgic-v3: add ICH_AP[01]Rn accessors for GICv3 wanghaibin 2017-07-06 16:18 ` Marc Zyngier 2017-07-07 9:22 ` wanghaibin 2017-07-05 11:23 ` [PATCH 3/4] kvm: arm/arm64: vgic: Implement the vGICv2 GICC_APRn uaccess interface wanghaibin 2017-07-06 16:53 ` Marc Zyngier 2017-07-07 8:28 ` wanghaibin 2017-07-05 11:23 ` [PATCH 4/4] kvm: arm/arm64: vgic: clean up vGICv3 ICC_APRn sysreg uaccess wanghaibin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.