On Fri, Jun 14, 2013 at 02:20:30PM +0000, Zhaobo (Bob, ERC) wrote:
> Hi all ,
Hi Bob,
First, to increase your chances of getting a response on these forums to
your questions, please consider using a decent e-mail client and send
your e-mails in clear text.
Your e-mail did not contain standard newline chars to wrap lines and
uses excessive indentation of the code.
> As introduced in gic arch spec , GICD_ICFGRn provide two bit(yet only
> bit[1] is used in gicv2) for each irq to indicate level or edge
> trigged irq type . One GICD_ICFGR can hold for 16 irqs. In order to
> save space , in software level , vgic_dist structure only use one bit
> for each irq type . so GICD_ICFGR[2n+1..2n] can be stored in one
> word(4 bytes).
Not sure exactly what your notation indicates here, but yes, by using
one bit instead of two bits, we can store the same information in half
the registers, roughly.
> In below function , offset is calculated with formula:
> offset = mmio->phys_addr - range->base - base . hereby offset of
> GICD_ICFGR1 is 4 and offset of GICD_ICFGR2 is 8. thus I think red
> parts in below function maybe is not correct. it would be changed to
>
>
> If ( (offset >> 2 )& 1)
> val = *reg >> 16;
> else
> val = *reg & 0xffff;
Yeah, you are right, it's a bug. The insight is simply that the
accessor is based on the real mmio offset, not the compressed version.
The patch below should fix it, thanks:
---
From: Christoffer Dall <christoffer.dall@xxxxxxxxxx>
Subject: [PATCH] KVM: ARM: vgic: Fix yet another cfg register bug
This code is so unnecessarily complicated, but it does serve the purpose
of being able to and all the bitmaps together in the end. Oh well, we
fix another issue.
The registers are accessed by the guests view of the real HW, not the
compressed backing view, so the offsets are one-off.
Reported-by: Zhaobo (Bob, ERC) <zhaobo@xxxxxxxxxx>
Signed-off-by: Christoffer Dall <christoffer.dall@xxxxxxxxxx>
---
arch/arm/kvm/vgic.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/kvm/vgic.c b/arch/arm/kvm/vgic.c
index 17c5ac7..981d448 100644
--- a/arch/arm/kvm/vgic.c
+++ b/arch/arm/kvm/vgic.c
@@ -549,7 +549,7 @@ static bool handle_mmio_cfg_reg(struct kvm_vcpu *vcpu,
u32 val;
u32 *reg = vgic_bitmap_get_reg(&vcpu->kvm->arch.vgic.irq_cfg,
vcpu->vcpu_id, offset >> 1);
- if (offset & 2)
+ if (offset & 4)
val = *reg >> 16;
else
val = *reg & 0xffff;
@@ -558,13 +558,13 @@ static bool handle_mmio_cfg_reg(struct kvm_vcpu *vcpu,
vgic_reg_access(mmio, &val, offset,
ACCESS_READ_VALUE | ACCESS_WRITE_VALUE);
if (mmio->is_write) {
- if (offset < 4) {
+ if (offset < 8) {
*reg = ~0U; /* Force PPIs/SGIs to 1 */
return false;
}
val = vgic_cfg_compress(val);
- if (offset & 2) {
+ if (offset & 4) {
*reg &= 0xffff;
*reg |= val << 16;
} else {
--
1.8.1.2
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
