diff mbox

[RFC,24/24] ARM: vGIC: advertising LPI support

Message ID 20160928182457.12433-25-andre.przywara@arm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andre Przywara Sept. 28, 2016, 6:24 p.m. UTC 
To let a guest know about the availability of virtual LPIs, set the
respective bits in the virtual GIC registers and let a guest control
the LPI enable bit.
Only report the LPI capability if the host has initialized at least
one ITS.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 xen/arch/arm/vgic-v3.c | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

Comments

Stefano Stabellini Nov. 10, 2016, 12:49 a.m. UTC | #1 
On Wed, 28 Sep 2016, Andre Przywara wrote:
> To let a guest know about the availability of virtual LPIs, set the
> respective bits in the virtual GIC registers and let a guest control
> the LPI enable bit.
> Only report the LPI capability if the host has initialized at least
> one ITS.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  xen/arch/arm/vgic-v3.c | 28 +++++++++++++++++++++++-----
>  1 file changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index d230a1f..61c97a2 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -168,8 +168,10 @@ static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info,
>      switch ( gicr_reg )
>      {
>      case VREG32(GICR_CTLR):
> -        /* We have not implemented LPI's, read zero */
> -        goto read_as_zero_32;
> +        if ( dabt.size != DABT_WORD ) goto bad_width;
> +        *r = vgic_reg32_extract(!!(v->arch.vgic.flags & VGIC_V3_LPIS_ENABLED),
> +                                info);

I don't think it is useful to call vgic_reg32_extract in this case.
vgic.flags is not a register.


> +        return 1;
>  
>      case VREG32(GICR_IIDR):
>          if ( dabt.size != DABT_WORD ) goto bad_width;
> @@ -181,16 +183,19 @@ static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info,
>          uint64_t typer, aff;
>  
>          if ( !vgic_reg64_check_access(dabt) ) goto bad_width;
> -        /* TBD: Update processor id in [23:8] when ITS support is added */
>          aff = (MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 3) << 56 |
>                 MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 2) << 48 |
>                 MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 1) << 40 |
>                 MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 0) << 32);
>          typer = aff;
> +        typer |= (v->vcpu_id & 0xffff) << 8;
>  
>          if ( v->arch.vgic.flags & VGIC_V3_RDIST_LAST )
>              typer |= GICR_TYPER_LAST;
>  
> +        if ( v->domain->arch.vgic.has_its )
> +            typer |= GICR_TYPER_PLPIS;
> +
>          *r = vgic_reg64_extract(typer, info);
>  
>          return 1;
> @@ -468,8 +473,16 @@ static int __vgic_v3_rdistr_rd_mmio_write(struct vcpu *v, mmio_info_t *info,
>      switch ( gicr_reg )
>      {
>      case VREG32(GICR_CTLR):
> -        /* LPI's not implemented */
> -        goto write_ignore_32;
> +        if ( dabt.size != DABT_WORD ) goto bad_width;
> +        if ( !v->domain->arch.vgic.has_its )
> +            return 1;
> +
> +        if ( r & 1 )
> +            v->arch.vgic.flags |= VGIC_V3_LPIS_ENABLED;
> +        else
> +            v->arch.vgic.flags &= !VGIC_V3_LPIS_ENABLED;
> +
> +        return 1;
>  
>      case VREG32(GICR_IIDR):
>          /* RO */
> @@ -1075,6 +1088,11 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
>          typer = ((ncpus - 1) << GICD_TYPE_CPUS_SHIFT |
>                   DIV_ROUND_UP(v->domain->arch.vgic.nr_spis, 32));
>  
> +        if ( v->domain->arch.vgic.has_its )
> +        {
> +            typer |= GICD_TYPE_LPIS;
> +            irq_bits = 16;
> +        }
>          typer |= (irq_bits - 1) << GICD_TYPE_ID_BITS_SHIFT;
>  
>          *r = vgic_reg32_extract(typer, info);
> -- 
> 2.9.0
>
Julien Grall Nov. 10, 2016, 11:22 a.m. UTC | #2 
On 10/11/16 00:49, Stefano Stabellini wrote:
> On Wed, 28 Sep 2016, Andre Przywara wrote:
>> To let a guest know about the availability of virtual LPIs, set the
>> respective bits in the virtual GIC registers and let a guest control
>> the LPI enable bit.
>> Only report the LPI capability if the host has initialized at least
>> one ITS.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> ---
>>  xen/arch/arm/vgic-v3.c | 28 +++++++++++++++++++++++-----
>>  1 file changed, 23 insertions(+), 5 deletions(-)
>>
>> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
>> index d230a1f..61c97a2 100644
>> --- a/xen/arch/arm/vgic-v3.c
>> +++ b/xen/arch/arm/vgic-v3.c
>> @@ -168,8 +168,10 @@ static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info,
>>      switch ( gicr_reg )
>>      {
>>      case VREG32(GICR_CTLR):
>> -        /* We have not implemented LPI's, read zero */
>> -        goto read_as_zero_32;
>> +        if ( dabt.size != DABT_WORD ) goto bad_width;
>> +        *r = vgic_reg32_extract(!!(v->arch.vgic.flags & VGIC_V3_LPIS_ENABLED),
>> +                                info);
>
> I don't think it is useful to call vgic_reg32_extract in this case.
> vgic.flags is not a register.

All the emulation is using vgic_reg*_extract and construct a register 
when necessary. So I would keep the vgic_reg32_extract.

However, it would be more readable to have:

uint32_t ctlr;

ctlr = !!(v->arch.vgic.flags & VGIC_V3_LPIS_ENABLED);
*r = vgic_reg32_extract(ctlr, info);

Regards,
diff mbox

Patch

diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index d230a1f..61c97a2 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -168,8 +168,10 @@  static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info,
     switch ( gicr_reg )
     {
     case VREG32(GICR_CTLR):
-        /* We have not implemented LPI's, read zero */
-        goto read_as_zero_32;
+        if ( dabt.size != DABT_WORD ) goto bad_width;
+        *r = vgic_reg32_extract(!!(v->arch.vgic.flags & VGIC_V3_LPIS_ENABLED),
+                                info);
+        return 1;
 
     case VREG32(GICR_IIDR):
         if ( dabt.size != DABT_WORD ) goto bad_width;
@@ -181,16 +183,19 @@  static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info,
         uint64_t typer, aff;
 
         if ( !vgic_reg64_check_access(dabt) ) goto bad_width;
-        /* TBD: Update processor id in [23:8] when ITS support is added */
         aff = (MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 3) << 56 |
                MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 2) << 48 |
                MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 1) << 40 |
                MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 0) << 32);
         typer = aff;
+        typer |= (v->vcpu_id & 0xffff) << 8;
 
         if ( v->arch.vgic.flags & VGIC_V3_RDIST_LAST )
             typer |= GICR_TYPER_LAST;
 
+        if ( v->domain->arch.vgic.has_its )
+            typer |= GICR_TYPER_PLPIS;
+
         *r = vgic_reg64_extract(typer, info);
 
         return 1;
@@ -468,8 +473,16 @@  static int __vgic_v3_rdistr_rd_mmio_write(struct vcpu *v, mmio_info_t *info,
     switch ( gicr_reg )
     {
     case VREG32(GICR_CTLR):
-        /* LPI's not implemented */
-        goto write_ignore_32;
+        if ( dabt.size != DABT_WORD ) goto bad_width;
+        if ( !v->domain->arch.vgic.has_its )
+            return 1;
+
+        if ( r & 1 )
+            v->arch.vgic.flags |= VGIC_V3_LPIS_ENABLED;
+        else
+            v->arch.vgic.flags &= !VGIC_V3_LPIS_ENABLED;
+
+        return 1;
 
     case VREG32(GICR_IIDR):
         /* RO */
@@ -1075,6 +1088,11 @@  static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
         typer = ((ncpus - 1) << GICD_TYPE_CPUS_SHIFT |
                  DIV_ROUND_UP(v->domain->arch.vgic.nr_spis, 32));
 
+        if ( v->domain->arch.vgic.has_its )
+        {
+            typer |= GICD_TYPE_LPIS;
+            irq_bits = 16;
+        }
         typer |= (irq_bits - 1) << GICD_TYPE_ID_BITS_SHIFT;
 
         *r = vgic_reg32_extract(typer, info);