Re: [RFC PATCH 24/33] irqchip/gic-v3-its: Add VPE scheduling

2017-06-19 Thread Marc Zyngier
On 14/02/17 00:13, Shanker Donthineni wrote:
> Hi Marc,
> 
> 
> On 01/17/2017 04:20 AM, Marc Zyngier wrote:
>> When a VPE is scheduled to run, the corresponding redistributor must
>> be told so, by setting VPROPBASER to the VM's property table, and
>> VPENDBASER to the vcpu's pending table.
>>
>> When scheduled out, we preserve the IDAI and PendingLast bits. The
>> latter is specially important, as it tells the hypervisor that
>> there are pending interrupts for this vcpu.
>>
>> Signed-off-by: Marc Zyngier 
>> ---
>>   drivers/irqchip/irq-gic-v3-its.c   | 57
>> ++
>>   include/linux/irqchip/arm-gic-v3.h | 63
>> ++
>>   2 files changed, 120 insertions(+)
>>
>> diff --git a/drivers/irqchip/irq-gic-v3-its.c
>> b/drivers/irqchip/irq-gic-v3-its.c
>> index 598e25b..f918d59 100644
>> --- a/drivers/irqchip/irq-gic-v3-its.c
>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>> @@ -143,6 +143,7 @@ static DEFINE_IDA(its_vpeid_ida);
>>   
>>   #define gic_data_rdist()   (raw_cpu_ptr(gic_rdists->rdist))
>>   #define gic_data_rdist_rd_base()   (gic_data_rdist()->rd_base)
>> +#define gic_data_rdist_vlpi_base()  (gic_data_rdist_rd_base() +
>> SZ_128K)
>>   
>>   static struct its_collection *dev_event_to_col(struct its_device
>> *its_dev,
>> u32 event)
>> @@ -2039,8 +2040,64 @@ static const struct irq_domain_ops its_domain_ops =
>> {
>>  .deactivate = its_irq_domain_deactivate,
>>   };
>>   
>> +static int its_vpe_set_vcpu_affinity(struct irq_data *d, void *vcpu_info)
>> +{
>> +struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
>> +struct its_cmd_info *info = vcpu_info;
>> +u64 val;
>> +
>> +switch (info->cmd_type) {
>> +case SCHEDULE_VPE:
>> +{
>> +void * __iomem vlpi_base = gic_data_rdist_vlpi_base();
>> +
>> +/* Schedule the VPE */
>> +val  = virt_to_phys(page_address(vpe->its_vm->vprop_page))
>> &
>> +GENMASK_ULL(51, 12);
>> +val |= (LPI_NRBITS - 1) & GICR_VPROPBASER_IDBITS_MASK;
>> +val |= GICR_VPROPBASER_RaWb;
>> +val |= GICR_VPROPBASER_InnerShareable;
>> +gits_write_vpropbaser(val, vlpi_base + GICR_VPROPBASER);
>> +
>> +val  = virt_to_phys(page_address(vpe->vpt_page)) &
>> GENMASK(51, 16);
>> +val |= GICR_VPENDBASER_WaWb;
>> +val |= GICR_VPENDBASER_NonShareable;
>> +val |= GICR_PENDBASER_PendingLast;
> Why always enabling PendingLast bit? You are keeping the pending last 
> bit in vpe->pending_last but not used here.

As I mentioned separately, this is wrong. You can race against an
interrupt being delivered and tell the redist that the pending table is
empty while there is actually something there.

>> +val |= vpe->idai ? GICR_PENDBASER_IDAI : 0;
>> +val |= GICR_PENDBASER_Valid;
> What is the status of the virtual CPU interface (ICH_HCR_EL2.En) at the 
> time of changing vPE state to resident? I believe the GICR will start 

That's not specified yet, and left to the hypervisor to decide. Yes, I
know that this is my left hand not knowing what my right hand is doing,
but I don't want to set things in stone at this stage.

> processing a pending table immediately after setting V=1 and with valid 
> PHYS address, GICR signals a virtual IRQ to virtual CPU if it has 
> outstanding interrupts. With my understanding of GIC spec, a virtual CPU 
> interface must be enabled before setting GICR_VPENDBASERR.Valid=1. 
> Otherwise, it leads to flooded vSet and Release messages between GICR 
> and CPU interface.
> 
> You can find Vset details in section 'A.4.18 VSet (IRI)' of GIC spec.
> 
> The CPU interface must Release an interrupt, ensuring that V == 1, if it 
> cannot handle the interrupt for either of the
> following reasons:
>   The interrupt group is disabled. This includes when the VM 
> interface is disabled, that is, when
> GICH_HCR.En or ICH_HCR.En, as appropriate, is cleared to 0.

I know that. But we also need to compensate for the time it take to read
the pending table. My current plan is to write to VPENDBASER as part of
the vgic setup, and let ICH_HCR_EL2.En be flipped on the normal path.

And frankly, who gives a damn if the redistributor and cpu-if are
playing ping-pong? At this stage, interrupts are disabled and we can't
handle anything...

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...


Re: [RFC PATCH 24/33] irqchip/gic-v3-its: Add VPE scheduling

2017-06-19 Thread Marc Zyngier
Coming back to this after spending too long doing something else...

On 16/03/17 21:41, Shanker Donthineni wrote:
> Hi Eric,
> 
> 
> On 03/16/2017 04:23 PM, Auger Eric wrote:
>> Hi,
>>
>> On 17/01/2017 11:20, Marc Zyngier wrote:
>>> When a VPE is scheduled to run, the corresponding redistributor must
>>> be told so, by setting VPROPBASER to the VM's property table, and
>>> VPENDBASER to the vcpu's pending table.
>>>
>>> When scheduled out, we preserve the IDAI and PendingLast bits. The
>>> latter is specially important, as it tells the hypervisor that
>>> there are pending interrupts for this vcpu.
>>>
>>> Signed-off-by: Marc Zyngier 
>>> ---
>>>  drivers/irqchip/irq-gic-v3-its.c   | 57 ++
>>>  include/linux/irqchip/arm-gic-v3.h | 63 
>>> ++
>>>  2 files changed, 120 insertions(+)
>>>
>>> diff --git a/drivers/irqchip/irq-gic-v3-its.c 
>>> b/drivers/irqchip/irq-gic-v3-its.c
>>> index 598e25b..f918d59 100644
>>> --- a/drivers/irqchip/irq-gic-v3-its.c
>>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>>> @@ -143,6 +143,7 @@ static DEFINE_IDA(its_vpeid_ida);
>>>  
>>>  #define gic_data_rdist()   (raw_cpu_ptr(gic_rdists->rdist))
>>>  #define gic_data_rdist_rd_base()   (gic_data_rdist()->rd_base)
>>> +#define gic_data_rdist_vlpi_base() (gic_data_rdist_rd_base() + SZ_128K)
>>>  
>>>  static struct its_collection *dev_event_to_col(struct its_device *its_dev,
>>>u32 event)
>>> @@ -2039,8 +2040,64 @@ static const struct irq_domain_ops its_domain_ops = {
>>> .deactivate = its_irq_domain_deactivate,
>>>  };
>>>  
>>> +static int its_vpe_set_vcpu_affinity(struct irq_data *d, void *vcpu_info)
>>> +{
>>> +   struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
>>> +   struct its_cmd_info *info = vcpu_info;
>>> +   u64 val;
>>> +
>>> +   switch (info->cmd_type) {
>>> +   case SCHEDULE_VPE:
>>> +   {
>>> +   void * __iomem vlpi_base = gic_data_rdist_vlpi_base();
>>> +
>>> +   /* Schedule the VPE */
>>> +   val  = virt_to_phys(page_address(vpe->its_vm->vprop_page)) &
>>> +   GENMASK_ULL(51, 12);
>>> +   val |= (LPI_NRBITS - 1) & GICR_VPROPBASER_IDBITS_MASK;
>>> +   val |= GICR_VPROPBASER_RaWb;
>>> +   val |= GICR_VPROPBASER_InnerShareable;
>>> +   gits_write_vpropbaser(val, vlpi_base + GICR_VPROPBASER);
>>> +
>>> +   val  = virt_to_phys(page_address(vpe->vpt_page)) & GENMASK(51, 
>>> 16);
>>> +   val |= GICR_VPENDBASER_WaWb;
>>> +   val |= GICR_VPENDBASER_NonShareable;
>>> +   val |= GICR_PENDBASER_PendingLast;
>> don't you want to restore the vpe->pending_last here? anyway I
>> understand this will force the HW to read the LPI pending table.
> 
> It's not a good idea to set PendLast bit always. There is no
> correctness issue but causes a huge impact on the system performance.
> No need to read pending table contents from memory if no VLPI are
> pending on vPE that is being scheduled.

Good idea or not, I don't believe you have a choice. You can be sure you
have pending VLPIs, but you cannot be sure you have none, as you can
easily race against the doorbell interrupt when entering the guest.

That's why we only use pending_last as a hint to tell the hypervisor
that the guest has a pending interrupt. You cannot use it as a hint to
the redistributor that it has no VLPIs.

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...


Re: [RFC PATCH 24/33] irqchip/gic-v3-its: Add VPE scheduling

2017-03-16 Thread Shanker Donthineni
Hi Eric,


On 03/16/2017 04:23 PM, Auger Eric wrote:
> Hi,
>
> On 17/01/2017 11:20, Marc Zyngier wrote:
>> When a VPE is scheduled to run, the corresponding redistributor must
>> be told so, by setting VPROPBASER to the VM's property table, and
>> VPENDBASER to the vcpu's pending table.
>>
>> When scheduled out, we preserve the IDAI and PendingLast bits. The
>> latter is specially important, as it tells the hypervisor that
>> there are pending interrupts for this vcpu.
>>
>> Signed-off-by: Marc Zyngier 
>> ---
>>  drivers/irqchip/irq-gic-v3-its.c   | 57 ++
>>  include/linux/irqchip/arm-gic-v3.h | 63 
>> ++
>>  2 files changed, 120 insertions(+)
>>
>> diff --git a/drivers/irqchip/irq-gic-v3-its.c 
>> b/drivers/irqchip/irq-gic-v3-its.c
>> index 598e25b..f918d59 100644
>> --- a/drivers/irqchip/irq-gic-v3-its.c
>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>> @@ -143,6 +143,7 @@ static DEFINE_IDA(its_vpeid_ida);
>>  
>>  #define gic_data_rdist()(raw_cpu_ptr(gic_rdists->rdist))
>>  #define gic_data_rdist_rd_base()(gic_data_rdist()->rd_base)
>> +#define gic_data_rdist_vlpi_base()  (gic_data_rdist_rd_base() + SZ_128K)
>>  
>>  static struct its_collection *dev_event_to_col(struct its_device *its_dev,
>> u32 event)
>> @@ -2039,8 +2040,64 @@ static const struct irq_domain_ops its_domain_ops = {
>>  .deactivate = its_irq_domain_deactivate,
>>  };
>>  
>> +static int its_vpe_set_vcpu_affinity(struct irq_data *d, void *vcpu_info)
>> +{
>> +struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
>> +struct its_cmd_info *info = vcpu_info;
>> +u64 val;
>> +
>> +switch (info->cmd_type) {
>> +case SCHEDULE_VPE:
>> +{
>> +void * __iomem vlpi_base = gic_data_rdist_vlpi_base();
>> +
>> +/* Schedule the VPE */
>> +val  = virt_to_phys(page_address(vpe->its_vm->vprop_page)) &
>> +GENMASK_ULL(51, 12);
>> +val |= (LPI_NRBITS - 1) & GICR_VPROPBASER_IDBITS_MASK;
>> +val |= GICR_VPROPBASER_RaWb;
>> +val |= GICR_VPROPBASER_InnerShareable;
>> +gits_write_vpropbaser(val, vlpi_base + GICR_VPROPBASER);
>> +
>> +val  = virt_to_phys(page_address(vpe->vpt_page)) & GENMASK(51, 
>> 16);
>> +val |= GICR_VPENDBASER_WaWb;
>> +val |= GICR_VPENDBASER_NonShareable;
>> +val |= GICR_PENDBASER_PendingLast;
> don't you want to restore the vpe->pending_last here? anyway I
> understand this will force the HW to read the LPI pending table.

It's not a good idea to set PendLast bit always. There is no correctness issue 
but causes a huge impact on the system performance. No need to read pending 
table contents from memory if no VLPI are pending on vPE that is being 
scheduled.

> Reviewed-by: Eric Auger 
>
> Eric
>
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

-- 
Shanker Donthineni
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux 
Foundation Collaborative Project.



Re: [RFC PATCH 24/33] irqchip/gic-v3-its: Add VPE scheduling

2017-03-16 Thread Auger Eric
Hi,

On 17/01/2017 11:20, Marc Zyngier wrote:
> When a VPE is scheduled to run, the corresponding redistributor must
> be told so, by setting VPROPBASER to the VM's property table, and
> VPENDBASER to the vcpu's pending table.
> 
> When scheduled out, we preserve the IDAI and PendingLast bits. The
> latter is specially important, as it tells the hypervisor that
> there are pending interrupts for this vcpu.
> 
> Signed-off-by: Marc Zyngier 
> ---
>  drivers/irqchip/irq-gic-v3-its.c   | 57 ++
>  include/linux/irqchip/arm-gic-v3.h | 63 
> ++
>  2 files changed, 120 insertions(+)
> 
> diff --git a/drivers/irqchip/irq-gic-v3-its.c 
> b/drivers/irqchip/irq-gic-v3-its.c
> index 598e25b..f918d59 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -143,6 +143,7 @@ static DEFINE_IDA(its_vpeid_ida);
>  
>  #define gic_data_rdist() (raw_cpu_ptr(gic_rdists->rdist))
>  #define gic_data_rdist_rd_base() (gic_data_rdist()->rd_base)
> +#define gic_data_rdist_vlpi_base()   (gic_data_rdist_rd_base() + SZ_128K)
>  
>  static struct its_collection *dev_event_to_col(struct its_device *its_dev,
>  u32 event)
> @@ -2039,8 +2040,64 @@ static const struct irq_domain_ops its_domain_ops = {
>   .deactivate = its_irq_domain_deactivate,
>  };
>  
> +static int its_vpe_set_vcpu_affinity(struct irq_data *d, void *vcpu_info)
> +{
> + struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
> + struct its_cmd_info *info = vcpu_info;
> + u64 val;
> +
> + switch (info->cmd_type) {
> + case SCHEDULE_VPE:
> + {
> + void * __iomem vlpi_base = gic_data_rdist_vlpi_base();
> +
> + /* Schedule the VPE */
> + val  = virt_to_phys(page_address(vpe->its_vm->vprop_page)) &
> + GENMASK_ULL(51, 12);
> + val |= (LPI_NRBITS - 1) & GICR_VPROPBASER_IDBITS_MASK;
> + val |= GICR_VPROPBASER_RaWb;
> + val |= GICR_VPROPBASER_InnerShareable;
> + gits_write_vpropbaser(val, vlpi_base + GICR_VPROPBASER);
> +
> + val  = virt_to_phys(page_address(vpe->vpt_page)) & GENMASK(51, 
> 16);
> + val |= GICR_VPENDBASER_WaWb;
> + val |= GICR_VPENDBASER_NonShareable;
> + val |= GICR_PENDBASER_PendingLast;
don't you want to restore the vpe->pending_last here? anyway I
understand this will force the HW to read the LPI pending table.

Reviewed-by: Eric Auger 

Eric

> + val |= vpe->idai ? GICR_PENDBASER_IDAI : 0;
> + val |= GICR_PENDBASER_Valid;
> + gits_write_vpendbaser(val, vlpi_base + GICR_VPENDBASER);
> +
> + return 0;
> + }
> +
> + case DESCHEDULE_VPE:
> + {
> + void * __iomem vlpi_base = gic_data_rdist_vlpi_base();
> +
> + /* We're being scheduled out */
> + val = gits_read_vpendbaser(vlpi_base + GICR_VPENDBASER);
> + val &= ~GICR_PENDBASER_Valid;
> + gits_write_vpendbaser(val, vlpi_base + GICR_VPENDBASER);
> +
> + val = gits_read_vpendbaser(vlpi_base + GICR_VPENDBASER);
> + while (val & GICR_PENDBASER_Dirty) {
> + cpu_relax();
> + val = gits_read_vpendbaser(vlpi_base + GICR_VPENDBASER);
> + }
> +
> + vpe->idai = !!(val & GICR_PENDBASER_IDAI);
> + vpe->pending_last = !!(val & GICR_PENDBASER_PendingLast);
> + return 0;
> + }
> +
> + default:
> + return -EINVAL;
> + }
> +}
> +
>  static struct irq_chip its_vpe_irq_chip = {
>   .name   = "GICv4-vpe",
> + .irq_set_vcpu_affinity  = its_vpe_set_vcpu_affinity,
>  };
>  
>  static int its_vpe_id_alloc(void)
> diff --git a/include/linux/irqchip/arm-gic-v3.h 
> b/include/linux/irqchip/arm-gic-v3.h
> index 1b3a070..2789c9a 100644
> --- a/include/linux/irqchip/arm-gic-v3.h
> +++ b/include/linux/irqchip/arm-gic-v3.h
> @@ -209,6 +209,69 @@
>  #define LPI_PROP_ENABLED (1 << 0)
>  
>  /*
> + * Re-Distributor registers, offsets from VLPI_base
> + */
> +#define GICR_VPROPBASER  0x0070
> +
> +#define GICR_VPROPBASER_IDBITS_MASK  0x1f
> +
> +#define GICR_VPROPBASER_SHAREABILITY_SHIFT   (10)
> +#define GICR_VPROPBASER_INNER_CACHEABILITY_SHIFT (7)
> +#define GICR_VPROPBASER_OUTER_CACHEABILITY_SHIFT (56)
> +
> +#define GICR_VPROPBASER_SHAREABILITY_MASK\
> + GIC_BASER_SHAREABILITY(GICR_VPROPBASER, SHAREABILITY_MASK)
> +#define GICR_VPROPBASER_INNER_CACHEABILITY_MASK  
> \
> + GIC_BASER_CACHEABILITY(GICR_VPROPBASER, INNER, MASK)
> +#define GICR_VPROPBASER_OUTER_CACHEABILITY_MASK  
> \
> + GIC_BASER_CACHEABILITY(GICR_VPROPBASER, OUTER, MASK)
> +#define GICR_VPROPBASER_CACHEABILITY_MASK  

Re: [RFC PATCH 24/33] irqchip/gic-v3-its: Add VPE scheduling

2017-02-13 Thread Shanker Donthineni

Hi Marc,

On 01/17/2017 04:20 AM, Marc Zyngier wrote:

When a VPE is scheduled to run, the corresponding redistributor must
be told so, by setting VPROPBASER to the VM's property table, and
VPENDBASER to the vcpu's pending table.

When scheduled out, we preserve the IDAI and PendingLast bits. The
latter is specially important, as it tells the hypervisor that
there are pending interrupts for this vcpu.

Signed-off-by: Marc Zyngier 
---
  drivers/irqchip/irq-gic-v3-its.c   | 57
++
  include/linux/irqchip/arm-gic-v3.h | 63
++
  2 files changed, 120 insertions(+)

diff --git a/drivers/irqchip/irq-gic-v3-its.c
b/drivers/irqchip/irq-gic-v3-its.c
index 598e25b..f918d59 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -143,6 +143,7 @@ static DEFINE_IDA(its_vpeid_ida);
  
  #define gic_data_rdist()		(raw_cpu_ptr(gic_rdists->rdist))

  #define gic_data_rdist_rd_base()  (gic_data_rdist()->rd_base)
+#define gic_data_rdist_vlpi_base() (gic_data_rdist_rd_base() +
SZ_128K)
  
  static struct its_collection *dev_event_to_col(struct its_device

*its_dev,
   u32 event)
@@ -2039,8 +2040,64 @@ static const struct irq_domain_ops its_domain_ops =
{
.deactivate = its_irq_domain_deactivate,
  };
  
+static int its_vpe_set_vcpu_affinity(struct irq_data *d, void *vcpu_info)

+{
+   struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
+   struct its_cmd_info *info = vcpu_info;
+   u64 val;
+
+   switch (info->cmd_type) {
+   case SCHEDULE_VPE:
+   {
+   void * __iomem vlpi_base = gic_data_rdist_vlpi_base();
+
+   /* Schedule the VPE */
+   val  = virt_to_phys(page_address(vpe->its_vm->vprop_page))
&
+   GENMASK_ULL(51, 12);
+   val |= (LPI_NRBITS - 1) & GICR_VPROPBASER_IDBITS_MASK;
+   val |= GICR_VPROPBASER_RaWb;
I know Write-allocation hints are not that important for PROP table, but 
like to know any reason why Wa hints are not enabled? Enable Ra if there 
is no strong reason behind it.

+   val |= GICR_VPROPBASER_InnerShareable;
+   gits_write_vpropbaser(val, vlpi_base + GICR_VPROPBASER);
+
+   val  = virt_to_phys(page_address(vpe->vpt_page)) &
GENMASK(51, 16);
+   val |= GICR_VPENDBASER_WaWb;
it's very important to enable read allocation hints for performance 
point of view. I think both Ra and Wa can be enabled without loosing any 
functionality.




+   val |= GICR_VPENDBASER_NonShareable;
+   val |= GICR_PENDBASER_PendingLast;
+   val |= vpe->idai ? GICR_PENDBASER_IDAI : 0;
+   val |= GICR_PENDBASER_Valid;
+   gits_write_vpendbaser(val, vlpi_base + GICR_VPENDBASER);
+
+   return 0;
+   }
+
+   case DESCHEDULE_VPE:
+   {
+   void * __iomem vlpi_base = gic_data_rdist_vlpi_base();
+
+   /* We're being scheduled out */
+   val = gits_read_vpendbaser(vlpi_base + GICR_VPENDBASER);
+   val &= ~GICR_PENDBASER_Valid;
+   gits_write_vpendbaser(val, vlpi_base + GICR_VPENDBASER);
+
+   val = gits_read_vpendbaser(vlpi_base + GICR_VPENDBASER);
+   while (val & GICR_PENDBASER_Dirty) {
+   cpu_relax();
+   val = gits_read_vpendbaser(vlpi_base +
GICR_VPENDBASER);
+   }
+
+   vpe->idai = !!(val & GICR_PENDBASER_IDAI);
+   vpe->pending_last = !!(val & GICR_PENDBASER_PendingLast);
+   return 0;
+   }
+
+   default:
+   return -EINVAL;
+   }
+}
+
  static struct irq_chip its_vpe_irq_chip = {
.name   = "GICv4-vpe",
+   .irq_set_vcpu_affinity  = its_vpe_set_vcpu_affinity,
  };
  
  static int its_vpe_id_alloc(void)

diff --git a/include/linux/irqchip/arm-gic-v3.h
b/include/linux/irqchip/arm-gic-v3.h
index 1b3a070..2789c9a 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -209,6 +209,69 @@
  #define LPI_PROP_ENABLED  (1 << 0)
  
  /*

+ * Re-Distributor registers, offsets from VLPI_base
+ */
+#define GICR_VPROPBASER0x0070
+
+#define GICR_VPROPBASER_IDBITS_MASK0x1f
+
+#define GICR_VPROPBASER_SHAREABILITY_SHIFT (10)
+#define GICR_VPROPBASER_INNER_CACHEABILITY_SHIFT   (7)
+#define GICR_VPROPBASER_OUTER_CACHEABILITY_SHIFT   (56)
+
+#define GICR_VPROPBASER_SHAREABILITY_MASK  \
+   GIC_BASER_SHAREABILITY(GICR_VPROPBASER, SHAREABILITY_MASK)
+#define GICR_VPROPBASER_INNER_CACHEABILITY_MASK
\
+   GIC_BASER_CACHEABILITY(GICR_VPROPBASER, INNER, MASK)
+#define GICR_VPROPBASER_OUTER_CACHEABILITY_MASK
\
+   GIC_BASER_CACHEABILITY(GICR_VPROPBASER, OUTER, MASK)
+#define GICR_VPROPBASER_CACHEABILIT

Re: [RFC PATCH 24/33] irqchip/gic-v3-its: Add VPE scheduling

2017-02-13 Thread Shanker Donthineni

Hi Marc,


On 01/17/2017 04:20 AM, Marc Zyngier wrote:

When a VPE is scheduled to run, the corresponding redistributor must
be told so, by setting VPROPBASER to the VM's property table, and
VPENDBASER to the vcpu's pending table.

When scheduled out, we preserve the IDAI and PendingLast bits. The
latter is specially important, as it tells the hypervisor that
there are pending interrupts for this vcpu.

Signed-off-by: Marc Zyngier 
---
  drivers/irqchip/irq-gic-v3-its.c   | 57
++
  include/linux/irqchip/arm-gic-v3.h | 63
++
  2 files changed, 120 insertions(+)

diff --git a/drivers/irqchip/irq-gic-v3-its.c
b/drivers/irqchip/irq-gic-v3-its.c
index 598e25b..f918d59 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -143,6 +143,7 @@ static DEFINE_IDA(its_vpeid_ida);
  
  #define gic_data_rdist()		(raw_cpu_ptr(gic_rdists->rdist))

  #define gic_data_rdist_rd_base()  (gic_data_rdist()->rd_base)
+#define gic_data_rdist_vlpi_base() (gic_data_rdist_rd_base() +
SZ_128K)
  
  static struct its_collection *dev_event_to_col(struct its_device

*its_dev,
   u32 event)
@@ -2039,8 +2040,64 @@ static const struct irq_domain_ops its_domain_ops =
{
.deactivate = its_irq_domain_deactivate,
  };
  
+static int its_vpe_set_vcpu_affinity(struct irq_data *d, void *vcpu_info)

+{
+   struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
+   struct its_cmd_info *info = vcpu_info;
+   u64 val;
+
+   switch (info->cmd_type) {
+   case SCHEDULE_VPE:
+   {
+   void * __iomem vlpi_base = gic_data_rdist_vlpi_base();
+
+   /* Schedule the VPE */
+   val  = virt_to_phys(page_address(vpe->its_vm->vprop_page))
&
+   GENMASK_ULL(51, 12);
+   val |= (LPI_NRBITS - 1) & GICR_VPROPBASER_IDBITS_MASK;
+   val |= GICR_VPROPBASER_RaWb;
+   val |= GICR_VPROPBASER_InnerShareable;
+   gits_write_vpropbaser(val, vlpi_base + GICR_VPROPBASER);
+
+   val  = virt_to_phys(page_address(vpe->vpt_page)) &
GENMASK(51, 16);
+   val |= GICR_VPENDBASER_WaWb;
+   val |= GICR_VPENDBASER_NonShareable;
+   val |= GICR_PENDBASER_PendingLast;
Why always enabling PendingLast bit? You are keeping the pending last 
bit in vpe->pending_last but not used here.

+   val |= vpe->idai ? GICR_PENDBASER_IDAI : 0;
+   val |= GICR_PENDBASER_Valid;
What is the status of the virtual CPU interface (ICH_HCR_EL2.En) at the 
time of changing vPE state to resident? I believe the GICR will start 
processing a pending table immediately after setting V=1 and with valid 
PHYS address, GICR signals a virtual IRQ to virtual CPU if it has 
outstanding interrupts. With my understanding of GIC spec, a virtual CPU 
interface must be enabled before setting GICR_VPENDBASERR.Valid=1. 
Otherwise, it leads to flooded vSet and Release messages between GICR 
and CPU interface.


You can find Vset details in section 'A.4.18 VSet (IRI)' of GIC spec.

The CPU interface must Release an interrupt, ensuring that V == 1, if it 
cannot handle the interrupt for either of the

following reasons:
 The interrupt group is disabled. This includes when the VM 
interface is disabled, that is, when

GICH_HCR.En or ICH_HCR.En, as appropriate, is cleared to 0.


+   gits_write_vpendbaser(val, vlpi_base + GICR_VPENDBASER);
+
+   return 0;
+   }
+
+   case DESCHEDULE_VPE:
+   {
+   void * __iomem vlpi_base = gic_data_rdist_vlpi_base();
+
+   /* We're being scheduled out */
+   val = gits_read_vpendbaser(vlpi_base + GICR_VPENDBASER);
+   val &= ~GICR_PENDBASER_Valid;
+   gits_write_vpendbaser(val, vlpi_base + GICR_VPENDBASER);
+
+   val = gits_read_vpendbaser(vlpi_base + GICR_VPENDBASER);
+   while (val & GICR_PENDBASER_Dirty) {
+   cpu_relax();
+   val = gits_read_vpendbaser(vlpi_base +
GICR_VPENDBASER);
+   }
+
+   vpe->idai = !!(val & GICR_PENDBASER_IDAI);
+   vpe->pending_last = !!(val & GICR_PENDBASER_PendingLast);
+   return 0;
+   }
+
+   default:
+   return -EINVAL;
+   }
+}
+
  static struct irq_chip its_vpe_irq_chip = {
.name   = "GICv4-vpe",
+   .irq_set_vcpu_affinity  = its_vpe_set_vcpu_affinity,
  };
  
  static int its_vpe_id_alloc(void)

diff --git a/include/linux/irqchip/arm-gic-v3.h
b/include/linux/irqchip/arm-gic-v3.h
index 1b3a070..2789c9a 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -209,6 +209,69 @@
  #define LPI_PROP_ENABLED  (1 << 0)
  
  /*

+ * Re-Distributor registers, offsets from VLPI_base
+ */
+#define GICR_VPROPB

Re: [RFC PATCH 24/33] irqchip/gic-v3-its: Add VPE scheduling

2017-02-13 Thread Thomas Gleixner
On Tue, 17 Jan 2017, Marc Zyngier wrote:
> +static int its_vpe_set_vcpu_affinity(struct irq_data *d, void *vcpu_info)
> +{
> + struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
> + struct its_cmd_info *info = vcpu_info;
> + u64 val;
> +
> + switch (info->cmd_type) {
> + case SCHEDULE_VPE:
> + {
> + void * __iomem vlpi_base = gic_data_rdist_vlpi_base();

Moving the vlpi_base line on top of the fucntion spares you lines and
brackets. No point in having the same thing twice

Thanks,

tglx