On 25/06/18 10:34, Christoffer Dall wrote:
> On Mon, Jun 25, 2018 at 09:19:07AM +0100, Marc Zyngier wrote:
>> On 24/06/18 23:11, Christoffer Dall wrote:
>>> Implement the required MMIO accessors for GICv2 and GICv3 for the
>>> IGROUPR distributor and redistributor registers.
>>>
>>> This can allow guests to change behavior compared to running on previous
>>> versions of KVM, but only to align with the architecture and hardware
>>> implementations.
>>>
>>> Signed-off-by: Christoffer Dall <christoffer.dall@xxxxxxx>
>>> ---
>>> virt/kvm/arm/vgic/vgic-mmio-v2.c | 2 +-
>>> virt/kvm/arm/vgic/vgic-mmio-v3.c | 2 +-
>>> virt/kvm/arm/vgic/vgic-mmio.c | 38 ++++++++++++++++++++++++++++++++
>>> virt/kvm/arm/vgic/vgic-mmio.h | 6 +++++
>>> 4 files changed, 46 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v2.c b/virt/kvm/arm/vgic/vgic-mmio-v2.c
>>> index 8d18f89397d3..ff3834d16ac9 100644
>>> --- a/virt/kvm/arm/vgic/vgic-mmio-v2.c
>>> +++ b/virt/kvm/arm/vgic/vgic-mmio-v2.c
>>> @@ -362,7 +362,7 @@ static const struct vgic_register_region vgic_v2_dist_registers[] = {
>>> vgic_mmio_read_v2_misc, vgic_mmio_write_v2_misc, 12,
>>> VGIC_ACCESS_32bit),
>>> REGISTER_DESC_WITH_BITS_PER_IRQ(GIC_DIST_IGROUP,
>>> - vgic_mmio_read_raz, vgic_mmio_write_wi, NULL, NULL, 1,
>>> + vgic_mmio_read_group, vgic_mmio_write_group, NULL, NULL, 1,
>>> VGIC_ACCESS_32bit),
>>> REGISTER_DESC_WITH_BITS_PER_IRQ(GIC_DIST_ENABLE_SET,
>>> vgic_mmio_read_enable, vgic_mmio_write_senable, NULL, NULL, 1,
>>> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
>>> index 287784095b5b..76e422859745 100644
>>> --- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
>>> +++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
>>> @@ -451,7 +451,7 @@ static const struct vgic_register_region vgic_v3_dist_registers[] = {
>>> vgic_mmio_read_rao, vgic_mmio_write_wi, 4,
>>> VGIC_ACCESS_32bit),
>>> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_IGROUPR,
>>> - vgic_mmio_read_rao, vgic_mmio_write_wi, NULL, NULL, 1,
>>> + vgic_mmio_read_group, vgic_mmio_write_group, NULL, NULL, 1,
>>> VGIC_ACCESS_32bit),
>>> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ISENABLER,
>>> vgic_mmio_read_enable, vgic_mmio_write_senable, NULL, NULL, 1,
>>
>> I think you're missing the GICR_IGROUPR accessor in the redistributor
>> (despite mentioning it in the commit message).
>>
>
> Indeed. I fixed this up on my kernel.org branch (vgic-group-fixes), and
> the fix is pretty trivial:
>
> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
> index 76e422859745..d2acad07dd30 100644
> --- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
> +++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
> @@ -524,7 +524,7 @@ static const struct vgic_register_region vgic_v3_rdbase_registers[] = {
>
> static const struct vgic_register_region vgic_v3_sgibase_registers[] = {
> REGISTER_DESC_WITH_LENGTH(GICR_IGROUPR0,
> - vgic_mmio_read_rao, vgic_mmio_write_wi, 4,
> + vgic_mmio_read_group, vgic_mmio_write_group, 4,
> VGIC_ACCESS_32bit),
> REGISTER_DESC_WITH_LENGTH(GICR_ISENABLER0,
> vgic_mmio_read_enable, vgic_mmio_write_senable, 4,
>
> I did a test on the model using both GICv3 and GICv2-on-GICv3, seems
> happy still.
>
> Do you want me to send a v2, or do you prefer to fix this up locally?
I can fix that myself. But this raises another question: As this change
is obviously observable by the guest, should we consider bumping up the
IIDR revision field?
Thanks,
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
