| Message ID | 20201212185010.26579-9-eric.auger@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM/ARM: Some vgic fixes and init sequence KVM selftests | expand |
Hi Eric, On 12/12/20 6:50 PM, Eric Auger wrote: > Commit 23bde34771f1 ("KVM: arm64: vgic-v3: Drop the > reporting of GICR_TYPER.Last for userspace") temporarily fixed > a bug identified when attempting to access the GICR_TYPER > register before the redistributor region setting but dropped > the support of the LAST bit. This patch restores its > support (if the redistributor region was set) while keeping the > code safe. I suppose the reason for emulating GICR_TYPER.Last is for architecture compliance, right? I think that should be in the commit message. > > Signed-off-by: Eric Auger <eric.auger@redhat.com> > --- > arch/arm64/kvm/vgic/vgic-mmio-v3.c | 7 ++++++- > include/kvm/arm_vgic.h | 1 + > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c > index 581f0f490000..2f9ef6058f6e 100644 > --- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c > +++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c > @@ -277,6 +277,8 @@ static unsigned long vgic_uaccess_read_v3r_typer(struct kvm_vcpu *vcpu, > gpa_t addr, unsigned int len) > { > unsigned long mpidr = kvm_vcpu_get_mpidr_aff(vcpu); > + struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu; > + struct vgic_redist_region *rdreg = vgic_cpu->rdreg; > int target_vcpu_id = vcpu->vcpu_id; > u64 value; > > @@ -286,7 +288,9 @@ static unsigned long vgic_uaccess_read_v3r_typer(struct kvm_vcpu *vcpu, > if (vgic_has_its(vcpu->kvm)) > value |= GICR_TYPER_PLPIS; > > - /* reporting of the Last bit is not supported for userspace */ > + if (rdreg && (vgic_cpu->rdreg_index == (rdreg->free_index - 1))) > + value |= GICR_TYPER_LAST; > + > return extract_bytes(value, addr & 7, len); > } > > @@ -714,6 +718,7 @@ int vgic_register_redist_iodev(struct kvm_vcpu *vcpu) > return -EINVAL; > > vgic_cpu->rdreg = rdreg; > + vgic_cpu->rdreg_index = rdreg->free_index; What happens if the next redistributor region we register has the base address adjacent to this one? I'm really not familiar with the code, but is it not possible to create two Redistributor regions (via KVM_DEV_ARM_VGIC_GRP_ADDR(KVM_VGIC_V3_ADDR_TYPE_REDIST)) where the second Redistributor region start address is immediately after the last Redistributor in the preceding region? Thanks, Alex > > rd_base = rdreg->base + rdreg->free_index * KVM_VGIC_V3_REDIST_SIZE; > > diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h > index a8d8fdcd3723..596c069263a7 100644 > --- a/include/kvm/arm_vgic.h > +++ b/include/kvm/arm_vgic.h > @@ -322,6 +322,7 @@ struct vgic_cpu { > */ > struct vgic_io_device rd_iodev; > struct vgic_redist_region *rdreg; > + u32 rdreg_index; > > /* Contains the attributes and gpa of the LPI pending tables. */ > u64 pendbaser;
Hi Eric, On 12/12/20 6:50 PM, Eric Auger wrote: > Commit 23bde34771f1 ("KVM: arm64: vgic-v3: Drop the > reporting of GICR_TYPER.Last for userspace") temporarily fixed > a bug identified when attempting to access the GICR_TYPER > register before the redistributor region setting but dropped > the support of the LAST bit. This patch restores its > support (if the redistributor region was set) while keeping the > code safe. If I understand your patch correctly, it is possible for the GICR_TYPER.Last bit to be transiently 1 if the register is accessed before all the redistributors regions have been configured. Arm IHI 0069F states that accesses to the GICR_TYPER register are RO. I haven't found exactly what RO means (please point me to the definition if you find it in the architecture!), but I assume it means read-only and I'm not sure how correct (from an architectural point of view) it is for two subsequent reads of this register to return different values. Maybe Marc can shed some light on this. Thanks, Alex > > Signed-off-by: Eric Auger <eric.auger@redhat.com> > --- > arch/arm64/kvm/vgic/vgic-mmio-v3.c | 7 ++++++- > include/kvm/arm_vgic.h | 1 + > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c > index 581f0f490000..2f9ef6058f6e 100644 > --- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c > +++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c > @@ -277,6 +277,8 @@ static unsigned long vgic_uaccess_read_v3r_typer(struct kvm_vcpu *vcpu, > gpa_t addr, unsigned int len) > { > unsigned long mpidr = kvm_vcpu_get_mpidr_aff(vcpu); > + struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu; > + struct vgic_redist_region *rdreg = vgic_cpu->rdreg; > int target_vcpu_id = vcpu->vcpu_id; > u64 value; > > @@ -286,7 +288,9 @@ static unsigned long vgic_uaccess_read_v3r_typer(struct kvm_vcpu *vcpu, > if (vgic_has_its(vcpu->kvm)) > value |= GICR_TYPER_PLPIS; > > - /* reporting of the Last bit is not supported for userspace */ > + if (rdreg && (vgic_cpu->rdreg_index == (rdreg->free_index - 1))) > + value |= GICR_TYPER_LAST; > + > return extract_bytes(value, addr & 7, len); > } > > @@ -714,6 +718,7 @@ int vgic_register_redist_iodev(struct kvm_vcpu *vcpu) > return -EINVAL; > > vgic_cpu->rdreg = rdreg; > + vgic_cpu->rdreg_index = rdreg->free_index; > > rd_base = rdreg->base + rdreg->free_index * KVM_VGIC_V3_REDIST_SIZE; > > diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h > index a8d8fdcd3723..596c069263a7 100644 > --- a/include/kvm/arm_vgic.h > +++ b/include/kvm/arm_vgic.h > @@ -322,6 +322,7 @@ struct vgic_cpu { > */ > struct vgic_io_device rd_iodev; > struct vgic_redist_region *rdreg; > + u32 rdreg_index; > > /* Contains the attributes and gpa of the LPI pending tables. */ > u64 pendbaser;
On 2021-01-12 17:28, Alexandru Elisei wrote: > Hi Eric, > > On 12/12/20 6:50 PM, Eric Auger wrote: >> Commit 23bde34771f1 ("KVM: arm64: vgic-v3: Drop the >> reporting of GICR_TYPER.Last for userspace") temporarily fixed >> a bug identified when attempting to access the GICR_TYPER >> register before the redistributor region setting but dropped >> the support of the LAST bit. This patch restores its >> support (if the redistributor region was set) while keeping the >> code safe. > > If I understand your patch correctly, it is possible for the > GICR_TYPER.Last bit > to be transiently 1 if the register is accessed before all the > redistributors > regions have been configured. > > Arm IHI 0069F states that accesses to the GICR_TYPER register are RO. I > haven't > found exactly what RO means (please point me to the definition if you > find it in > the architecture!), but I assume it means read-only and I'm not sure > how correct > (from an architectural point of view) it is for two subsequent reads of > this > register to return different values. Maybe Marc can shed some light on > this. RO = Read-Only indeed. Not sure that's documented anywhere in the architecture, but this is enough of a well known acronym that even the ARM ARM doesn't feel the need to invent a new definition for it. As for your concern, I don't think it is a problem to return different values if the HW has changed in between. This may need to be documented though. Thanks, M.
Hi Alexandru, On 1/12/21 6:02 PM, Alexandru Elisei wrote: > Hi Eric, > > On 12/12/20 6:50 PM, Eric Auger wrote: >> Commit 23bde34771f1 ("KVM: arm64: vgic-v3: Drop the >> reporting of GICR_TYPER.Last for userspace") temporarily fixed >> a bug identified when attempting to access the GICR_TYPER >> register before the redistributor region setting but dropped >> the support of the LAST bit. This patch restores its >> support (if the redistributor region was set) while keeping the >> code safe. > > I suppose the reason for emulating GICR_TYPER.Last is for architecture compliance, > right? I think that should be in the commit message. OK added this in the commit msg. > >> >> Signed-off-by: Eric Auger <eric.auger@redhat.com> >> --- >> arch/arm64/kvm/vgic/vgic-mmio-v3.c | 7 ++++++- >> include/kvm/arm_vgic.h | 1 + >> 2 files changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c >> index 581f0f490000..2f9ef6058f6e 100644 >> --- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c >> +++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c >> @@ -277,6 +277,8 @@ static unsigned long vgic_uaccess_read_v3r_typer(struct kvm_vcpu *vcpu, >> gpa_t addr, unsigned int len) >> { >> unsigned long mpidr = kvm_vcpu_get_mpidr_aff(vcpu); >> + struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu; >> + struct vgic_redist_region *rdreg = vgic_cpu->rdreg; >> int target_vcpu_id = vcpu->vcpu_id; >> u64 value; >> >> @@ -286,7 +288,9 @@ static unsigned long vgic_uaccess_read_v3r_typer(struct kvm_vcpu *vcpu, >> if (vgic_has_its(vcpu->kvm)) >> value |= GICR_TYPER_PLPIS; >> >> - /* reporting of the Last bit is not supported for userspace */ >> + if (rdreg && (vgic_cpu->rdreg_index == (rdreg->free_index - 1))) >> + value |= GICR_TYPER_LAST; >> + >> return extract_bytes(value, addr & 7, len); >> } >> >> @@ -714,6 +718,7 @@ int vgic_register_redist_iodev(struct kvm_vcpu *vcpu) >> return -EINVAL; >> >> vgic_cpu->rdreg = rdreg; >> + vgic_cpu->rdreg_index = rdreg->free_index; > > What happens if the next redistributor region we register has the base address > adjacent to this one? > > I'm really not familiar with the code, but is it not possible to create two > Redistributor regions (via > KVM_DEV_ARM_VGIC_GRP_ADDR(KVM_VGIC_V3_ADDR_TYPE_REDIST)) where the second > Redistributor region start address is immediately after the last Redistributor in > the preceding region? KVM_VGIC_V3_ADDR_TYPE_REDIST only allows to create a single rdist region. Only KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION allows to register several of them. with KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION, it is possible to register adjacent rdist regions. vgic_v3_rdist_free_slot() previously returned the 1st rdist region where enough space remains for inserting the new reg. We put the rdist at the free index there. But maybe I misunderstood your question? Thanks Eric > > Thanks, > Alex >> >> rd_base = rdreg->base + rdreg->free_index * KVM_VGIC_V3_REDIST_SIZE; >> >> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h >> index a8d8fdcd3723..596c069263a7 100644 >> --- a/include/kvm/arm_vgic.h >> +++ b/include/kvm/arm_vgic.h >> @@ -322,6 +322,7 @@ struct vgic_cpu { >> */ >> struct vgic_io_device rd_iodev; >> struct vgic_redist_region *rdreg; >> + u32 rdreg_index; >> >> /* Contains the attributes and gpa of the LPI pending tables. */ >> u64 pendbaser; >
Hi Eric, On 1/14/21 10:16 AM, Auger Eric wrote: > Hi Alexandru, > > On 1/12/21 6:02 PM, Alexandru Elisei wrote: >> Hi Eric, >> >> On 12/12/20 6:50 PM, Eric Auger wrote: >>> Commit 23bde34771f1 ("KVM: arm64: vgic-v3: Drop the >>> reporting of GICR_TYPER.Last for userspace") temporarily fixed >>> a bug identified when attempting to access the GICR_TYPER >>> register before the redistributor region setting but dropped >>> the support of the LAST bit. This patch restores its >>> support (if the redistributor region was set) while keeping the >>> code safe. >> I suppose the reason for emulating GICR_TYPER.Last is for architecture compliance, >> right? I think that should be in the commit message. > OK added this in the commit msg. >>> Signed-off-by: Eric Auger <eric.auger@redhat.com> >>> --- >>> arch/arm64/kvm/vgic/vgic-mmio-v3.c | 7 ++++++- >>> include/kvm/arm_vgic.h | 1 + >>> 2 files changed, 7 insertions(+), 1 deletion(-) >>> >>> diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c >>> index 581f0f490000..2f9ef6058f6e 100644 >>> --- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c >>> +++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c >>> @@ -277,6 +277,8 @@ static unsigned long vgic_uaccess_read_v3r_typer(struct kvm_vcpu *vcpu, >>> gpa_t addr, unsigned int len) >>> { >>> unsigned long mpidr = kvm_vcpu_get_mpidr_aff(vcpu); >>> + struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu; >>> + struct vgic_redist_region *rdreg = vgic_cpu->rdreg; >>> int target_vcpu_id = vcpu->vcpu_id; >>> u64 value; >>> >>> @@ -286,7 +288,9 @@ static unsigned long vgic_uaccess_read_v3r_typer(struct kvm_vcpu *vcpu, >>> if (vgic_has_its(vcpu->kvm)) >>> value |= GICR_TYPER_PLPIS; >>> >>> - /* reporting of the Last bit is not supported for userspace */ >>> + if (rdreg && (vgic_cpu->rdreg_index == (rdreg->free_index - 1))) >>> + value |= GICR_TYPER_LAST; >>> + >>> return extract_bytes(value, addr & 7, len); >>> } >>> >>> @@ -714,6 +718,7 @@ int vgic_register_redist_iodev(struct kvm_vcpu *vcpu) >>> return -EINVAL; >>> >>> vgic_cpu->rdreg = rdreg; >>> + vgic_cpu->rdreg_index = rdreg->free_index; >> What happens if the next redistributor region we register has the base address >> adjacent to this one? >> >> I'm really not familiar with the code, but is it not possible to create two >> Redistributor regions (via >> KVM_DEV_ARM_VGIC_GRP_ADDR(KVM_VGIC_V3_ADDR_TYPE_REDIST)) where the second >> Redistributor region start address is immediately after the last Redistributor in >> the preceding region? > KVM_VGIC_V3_ADDR_TYPE_REDIST only allows to create a single rdist > region. Only KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION allows to register > several of them. > > with KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION, it is possible to register > adjacent rdist regions. vgic_v3_rdist_free_slot() previously returned > the 1st rdist region where enough space remains for inserting the new > reg. We put the rdist at the free index there. > > But maybe I misunderstood your question? Yes, I think you did a good job at answering my poorly worded question. This is the case I am concerned about: 1. Userspace sets first redistributor base address to 0x0 via KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION(count = 1, base = 0x0, flags = 0, index = 0). 2. Userspace sets first redistributor base address to 0x0 + 128K, immediately following the previous Redistributor. In that case the two Redistributors will be represented by two separate struct vgic_redist_region, but they are adjacent to one another and represent one contiguous memory region. From what I understand from your patch, GICR_TYPER.Last will be set for both Redistributors, when it should be set only for the second Redistributor. Does any of that make sense? Thanks, Alex > > Thanks > > Eric >> Thanks, >> Alex >>> >>> rd_base = rdreg->base + rdreg->free_index * KVM_VGIC_V3_REDIST_SIZE; >>> >>> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h >>> index a8d8fdcd3723..596c069263a7 100644 >>> --- a/include/kvm/arm_vgic.h >>> +++ b/include/kvm/arm_vgic.h >>> @@ -322,6 +322,7 @@ struct vgic_cpu { >>> */ >>> struct vgic_io_device rd_iodev; >>> struct vgic_redist_region *rdreg; >>> + u32 rdreg_index; >>> >>> /* Contains the attributes and gpa of the LPI pending tables. */ >>> u64 pendbaser;
Hi Alexandru, On 1/20/21 5:13 PM, Alexandru Elisei wrote: > Hi Eric, > > On 1/14/21 10:16 AM, Auger Eric wrote: >> Hi Alexandru, >> >> On 1/12/21 6:02 PM, Alexandru Elisei wrote: >>> Hi Eric, >>> >>> On 12/12/20 6:50 PM, Eric Auger wrote: >>>> Commit 23bde34771f1 ("KVM: arm64: vgic-v3: Drop the >>>> reporting of GICR_TYPER.Last for userspace") temporarily fixed >>>> a bug identified when attempting to access the GICR_TYPER >>>> register before the redistributor region setting but dropped >>>> the support of the LAST bit. This patch restores its >>>> support (if the redistributor region was set) while keeping the >>>> code safe. >>> I suppose the reason for emulating GICR_TYPER.Last is for architecture compliance, >>> right? I think that should be in the commit message. >> OK added this in the commit msg. >>>> Signed-off-by: Eric Auger <eric.auger@redhat.com> >>>> --- >>>> arch/arm64/kvm/vgic/vgic-mmio-v3.c | 7 ++++++- >>>> include/kvm/arm_vgic.h | 1 + >>>> 2 files changed, 7 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c >>>> index 581f0f490000..2f9ef6058f6e 100644 >>>> --- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c >>>> +++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c >>>> @@ -277,6 +277,8 @@ static unsigned long vgic_uaccess_read_v3r_typer(struct kvm_vcpu *vcpu, >>>> gpa_t addr, unsigned int len) >>>> { >>>> unsigned long mpidr = kvm_vcpu_get_mpidr_aff(vcpu); >>>> + struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu; >>>> + struct vgic_redist_region *rdreg = vgic_cpu->rdreg; >>>> int target_vcpu_id = vcpu->vcpu_id; >>>> u64 value; >>>> >>>> @@ -286,7 +288,9 @@ static unsigned long vgic_uaccess_read_v3r_typer(struct kvm_vcpu *vcpu, >>>> if (vgic_has_its(vcpu->kvm)) >>>> value |= GICR_TYPER_PLPIS; >>>> >>>> - /* reporting of the Last bit is not supported for userspace */ >>>> + if (rdreg && (vgic_cpu->rdreg_index == (rdreg->free_index - 1))) >>>> + value |= GICR_TYPER_LAST; >>>> + >>>> return extract_bytes(value, addr & 7, len); >>>> } >>>> >>>> @@ -714,6 +718,7 @@ int vgic_register_redist_iodev(struct kvm_vcpu *vcpu) >>>> return -EINVAL; >>>> >>>> vgic_cpu->rdreg = rdreg; >>>> + vgic_cpu->rdreg_index = rdreg->free_index; >>> What happens if the next redistributor region we register has the base address >>> adjacent to this one? >>> >>> I'm really not familiar with the code, but is it not possible to create two >>> Redistributor regions (via >>> KVM_DEV_ARM_VGIC_GRP_ADDR(KVM_VGIC_V3_ADDR_TYPE_REDIST)) where the second >>> Redistributor region start address is immediately after the last Redistributor in >>> the preceding region? >> KVM_VGIC_V3_ADDR_TYPE_REDIST only allows to create a single rdist >> region. Only KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION allows to register >> several of them. >> >> with KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION, it is possible to register >> adjacent rdist regions. vgic_v3_rdist_free_slot() previously returned >> the 1st rdist region where enough space remains for inserting the new >> reg. We put the rdist at the free index there. >> >> But maybe I misunderstood your question? > > Yes, I think you did a good job at answering my poorly worded question. > > This is the case I am concerned about: > > 1. Userspace sets first redistributor base address to 0x0 via > KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION(count = 1, base = 0x0, flags = 0, index = 0). > > 2. Userspace sets first redistributor base address to 0x0 + 128K, immediately > following the previous Redistributor. > > In that case the two Redistributors will be represented by two separate struct > vgic_redist_region, but they are adjacent to one another and represent one > contiguous memory region. > > From what I understand from your patch, GICR_TYPER.Last will be set for both > Redistributors, when it should be set only for the second Redistributor. Does any > of that make sense? Please forgive me for not having replied before on this thread. This is a valid concern. Nothing prevents the redistributor regions from being contiguous although this is not the goal. Also nothing prevents vcpu rdists to be laid out within a redist region in non ascending order. Also redist regions with ascending indices may not have increasing base addresses. So this becomes a gas factory for emulating a single bit but I have reworked this in v3 ;-) Thanks Eric > > Thanks, > Alex >> >> Thanks >> >> Eric >>> Thanks, >>> Alex >>>> >>>> rd_base = rdreg->base + rdreg->free_index * KVM_VGIC_V3_REDIST_SIZE; >>>> >>>> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h >>>> index a8d8fdcd3723..596c069263a7 100644 >>>> --- a/include/kvm/arm_vgic.h >>>> +++ b/include/kvm/arm_vgic.h >>>> @@ -322,6 +322,7 @@ struct vgic_cpu { >>>> */ >>>> struct vgic_io_device rd_iodev; >>>> struct vgic_redist_region *rdreg; >>>> + u32 rdreg_index; >>>> >>>> /* Contains the attributes and gpa of the LPI pending tables. */ >>>> u64 pendbaser; >
diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c index 581f0f490000..2f9ef6058f6e 100644 --- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c +++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c @@ -277,6 +277,8 @@ static unsigned long vgic_uaccess_read_v3r_typer(struct kvm_vcpu *vcpu, gpa_t addr, unsigned int len) { unsigned long mpidr = kvm_vcpu_get_mpidr_aff(vcpu); + struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu; + struct vgic_redist_region *rdreg = vgic_cpu->rdreg; int target_vcpu_id = vcpu->vcpu_id; u64 value; @@ -286,7 +288,9 @@ static unsigned long vgic_uaccess_read_v3r_typer(struct kvm_vcpu *vcpu, if (vgic_has_its(vcpu->kvm)) value |= GICR_TYPER_PLPIS; - /* reporting of the Last bit is not supported for userspace */ + if (rdreg && (vgic_cpu->rdreg_index == (rdreg->free_index - 1))) + value |= GICR_TYPER_LAST; + return extract_bytes(value, addr & 7, len); } @@ -714,6 +718,7 @@ int vgic_register_redist_iodev(struct kvm_vcpu *vcpu) return -EINVAL; vgic_cpu->rdreg = rdreg; + vgic_cpu->rdreg_index = rdreg->free_index; rd_base = rdreg->base + rdreg->free_index * KVM_VGIC_V3_REDIST_SIZE; diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h index a8d8fdcd3723..596c069263a7 100644 --- a/include/kvm/arm_vgic.h +++ b/include/kvm/arm_vgic.h @@ -322,6 +322,7 @@ struct vgic_cpu { */ struct vgic_io_device rd_iodev; struct vgic_redist_region *rdreg; + u32 rdreg_index; /* Contains the attributes and gpa of the LPI pending tables. */ u64 pendbaser;
Commit 23bde34771f1 ("KVM: arm64: vgic-v3: Drop the reporting of GICR_TYPER.Last for userspace") temporarily fixed a bug identified when attempting to access the GICR_TYPER register before the redistributor region setting but dropped the support of the LAST bit. This patch restores its support (if the redistributor region was set) while keeping the code safe. Signed-off-by: Eric Auger <eric.auger@redhat.com> --- arch/arm64/kvm/vgic/vgic-mmio-v3.c | 7 ++++++- include/kvm/arm_vgic.h | 1 + 2 files changed, 7 insertions(+), 1 deletion(-)