The GICv4.1 ITS has yet another new command (VSGI) which allows
a VPE-targeted SGI to be configured (or have its pending state
cleared). Add support for this command and plumb it into the
activate irqdomain callback so that it is ready to be used.
Signed-off-by: Marc Zyngier <[email protected]>
Reviewed-by: Zenghui Yu <[email protected]>
---
drivers/irqchip/irq-gic-v3-its.c | 79 +++++++++++++++++++++++++++++-
include/linux/irqchip/arm-gic-v3.h | 3 +-
2 files changed, 80 insertions(+), 2 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 112b452fcb40..e0db3f906f87 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -380,6 +380,15 @@ struct its_cmd_desc {
struct {
struct its_vpe *vpe;
} its_invdb_cmd;
+
+ struct {
+ struct its_vpe *vpe;
+ u8 sgi;
+ u8 priority;
+ bool enable;
+ bool group;
+ bool clear;
+ } its_vsgi_cmd;
};
};
@@ -528,6 +537,31 @@ static void its_encode_db(struct its_cmd_block *cmd, bool db)
its_mask_encode(&cmd->raw_cmd[2], db, 63, 63);
}
+static void its_encode_sgi_intid(struct its_cmd_block *cmd, u8 sgi)
+{
+ its_mask_encode(&cmd->raw_cmd[0], sgi, 35, 32);
+}
+
+static void its_encode_sgi_priority(struct its_cmd_block *cmd, u8 prio)
+{
+ its_mask_encode(&cmd->raw_cmd[0], prio >> 4, 23, 20);
+}
+
+static void its_encode_sgi_group(struct its_cmd_block *cmd, bool grp)
+{
+ its_mask_encode(&cmd->raw_cmd[0], grp, 10, 10);
+}
+
+static void its_encode_sgi_clear(struct its_cmd_block *cmd, bool clr)
+{
+ its_mask_encode(&cmd->raw_cmd[0], clr, 9, 9);
+}
+
+static void its_encode_sgi_enable(struct its_cmd_block *cmd, bool en)
+{
+ its_mask_encode(&cmd->raw_cmd[0], en, 8, 8);
+}
+
static inline void its_fixup_cmd(struct its_cmd_block *cmd)
{
/* Let's fixup BE commands */
@@ -893,6 +927,26 @@ static struct its_vpe *its_build_invdb_cmd(struct its_node *its,
return valid_vpe(its, desc->its_invdb_cmd.vpe);
}
+static struct its_vpe *its_build_vsgi_cmd(struct its_node *its,
+ struct its_cmd_block *cmd,
+ struct its_cmd_desc *desc)
+{
+ if (WARN_ON(!is_v4_1(its)))
+ return NULL;
+
+ its_encode_cmd(cmd, GITS_CMD_VSGI);
+ its_encode_vpeid(cmd, desc->its_vsgi_cmd.vpe->vpe_id);
+ its_encode_sgi_intid(cmd, desc->its_vsgi_cmd.sgi);
+ its_encode_sgi_priority(cmd, desc->its_vsgi_cmd.priority);
+ its_encode_sgi_group(cmd, desc->its_vsgi_cmd.group);
+ its_encode_sgi_clear(cmd, desc->its_vsgi_cmd.clear);
+ its_encode_sgi_enable(cmd, desc->its_vsgi_cmd.enable);
+
+ its_fixup_cmd(cmd);
+
+ return valid_vpe(its, desc->its_vsgi_cmd.vpe);
+}
+
static u64 its_cmd_ptr_to_offset(struct its_node *its,
struct its_cmd_block *ptr)
{
@@ -3870,6 +3924,21 @@ static struct irq_chip its_vpe_4_1_irq_chip = {
.irq_set_vcpu_affinity = its_vpe_4_1_set_vcpu_affinity,
};
+static void its_configure_sgi(struct irq_data *d, bool clear)
+{
+ struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
+ struct its_cmd_desc desc;
+
+ desc.its_vsgi_cmd.vpe = vpe;
+ desc.its_vsgi_cmd.sgi = d->hwirq;
+ desc.its_vsgi_cmd.priority = vpe->sgi_config[d->hwirq].priority;
+ desc.its_vsgi_cmd.enable = vpe->sgi_config[d->hwirq].enabled;
+ desc.its_vsgi_cmd.group = vpe->sgi_config[d->hwirq].group;
+ desc.its_vsgi_cmd.clear = clear;
+
+ its_send_single_vcommand(find_4_1_its(), its_build_vsgi_cmd, &desc);
+}
+
static int its_sgi_set_affinity(struct irq_data *d,
const struct cpumask *mask_val,
bool force)
@@ -3915,13 +3984,21 @@ static void its_sgi_irq_domain_free(struct irq_domain *domain,
static int its_sgi_irq_domain_activate(struct irq_domain *domain,
struct irq_data *d, bool reserve)
{
+ /* Write out the initial SGI configuration */
+ its_configure_sgi(d, false);
return 0;
}
static void its_sgi_irq_domain_deactivate(struct irq_domain *domain,
struct irq_data *d)
{
- /* Nothing to do */
+ struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
+
+ /* First disable the SGI */
+ vpe->sgi_config[d->hwirq].enabled = false;
+ its_configure_sgi(d, false);
+ /* Now clear the potential pending bit (yes, this is clunky) */
+ its_configure_sgi(d, true);
}
static struct irq_domain_ops its_sgi_domain_ops = {
diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
index b28acfa71f82..fd3be49ac9a5 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -502,8 +502,9 @@
#define GITS_CMD_VMAPTI GITS_CMD_GICv4(GITS_CMD_MAPTI)
#define GITS_CMD_VMOVI GITS_CMD_GICv4(GITS_CMD_MOVI)
#define GITS_CMD_VSYNC GITS_CMD_GICv4(GITS_CMD_SYNC)
-/* VMOVP and INVDB are the odd ones, as they dont have a physical counterpart */
+/* VMOVP, VSGI and INVDB are the odd ones, as they dont have a physical counterpart */
#define GITS_CMD_VMOVP GITS_CMD_GICv4(2)
+#define GITS_CMD_VSGI GITS_CMD_GICv4(3)
#define GITS_CMD_INVDB GITS_CMD_GICv4(0xe)
/*
--
2.20.1
Hi Marc,
On 3/4/20 9:33 PM, Marc Zyngier wrote:
> The GICv4.1 ITS has yet another new command (VSGI) which allows
> a VPE-targeted SGI to be configured (or have its pending state
> cleared). Add support for this command and plumb it into the
> activate irqdomain callback so that it is ready to be used.
>
> Signed-off-by: Marc Zyngier <[email protected]>
> Reviewed-by: Zenghui Yu <[email protected]>
> ---
> drivers/irqchip/irq-gic-v3-its.c | 79 +++++++++++++++++++++++++++++-
> include/linux/irqchip/arm-gic-v3.h | 3 +-
> 2 files changed, 80 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 112b452fcb40..e0db3f906f87 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -380,6 +380,15 @@ struct its_cmd_desc {
> struct {
> struct its_vpe *vpe;
> } its_invdb_cmd;
> +
> + struct {
> + struct its_vpe *vpe;
> + u8 sgi;
> + u8 priority;
> + bool enable;
> + bool group;
> + bool clear;
> + } its_vsgi_cmd;
> };
> };
>
> @@ -528,6 +537,31 @@ static void its_encode_db(struct its_cmd_block *cmd, bool db)
> its_mask_encode(&cmd->raw_cmd[2], db, 63, 63);
> }
>
> +static void its_encode_sgi_intid(struct its_cmd_block *cmd, u8 sgi)
> +{
> + its_mask_encode(&cmd->raw_cmd[0], sgi, 35, 32);
> +}
> +
> +static void its_encode_sgi_priority(struct its_cmd_block *cmd, u8 prio)
> +{
> + its_mask_encode(&cmd->raw_cmd[0], prio >> 4, 23, 20);
> +}
> +
> +static void its_encode_sgi_group(struct its_cmd_block *cmd, bool grp)
> +{
> + its_mask_encode(&cmd->raw_cmd[0], grp, 10, 10);
> +}
> +
> +static void its_encode_sgi_clear(struct its_cmd_block *cmd, bool clr)
> +{
> + its_mask_encode(&cmd->raw_cmd[0], clr, 9, 9);
> +}
> +
> +static void its_encode_sgi_enable(struct its_cmd_block *cmd, bool en)
> +{
> + its_mask_encode(&cmd->raw_cmd[0], en, 8, 8);
> +}
> +
> static inline void its_fixup_cmd(struct its_cmd_block *cmd)
> {
> /* Let's fixup BE commands */
> @@ -893,6 +927,26 @@ static struct its_vpe *its_build_invdb_cmd(struct its_node *its,
> return valid_vpe(its, desc->its_invdb_cmd.vpe);
> }
>
> +static struct its_vpe *its_build_vsgi_cmd(struct its_node *its,
> + struct its_cmd_block *cmd,
> + struct its_cmd_desc *desc)
> +{
> + if (WARN_ON(!is_v4_1(its)))
> + return NULL;
> +
> + its_encode_cmd(cmd, GITS_CMD_VSGI);
> + its_encode_vpeid(cmd, desc->its_vsgi_cmd.vpe->vpe_id);
> + its_encode_sgi_intid(cmd, desc->its_vsgi_cmd.sgi);
> + its_encode_sgi_priority(cmd, desc->its_vsgi_cmd.priority);
> + its_encode_sgi_group(cmd, desc->its_vsgi_cmd.group);
> + its_encode_sgi_clear(cmd, desc->its_vsgi_cmd.clear);
> + its_encode_sgi_enable(cmd, desc->its_vsgi_cmd.enable);
> +
> + its_fixup_cmd(cmd);
> +
> + return valid_vpe(its, desc->its_vsgi_cmd.vpe);
> +}
> +
> static u64 its_cmd_ptr_to_offset(struct its_node *its,
> struct its_cmd_block *ptr)
> {
> @@ -3870,6 +3924,21 @@ static struct irq_chip its_vpe_4_1_irq_chip = {
> .irq_set_vcpu_affinity = its_vpe_4_1_set_vcpu_affinity,
> };
>
> +static void its_configure_sgi(struct irq_data *d, bool clear)
> +{
> + struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
> + struct its_cmd_desc desc;
> +
> + desc.its_vsgi_cmd.vpe = vpe;
> + desc.its_vsgi_cmd.sgi = d->hwirq;
> + desc.its_vsgi_cmd.priority = vpe->sgi_config[d->hwirq].priority;
> + desc.its_vsgi_cmd.enable = vpe->sgi_config[d->hwirq].enabled;
> + desc.its_vsgi_cmd.group = vpe->sgi_config[d->hwirq].group;
> + desc.its_vsgi_cmd.clear = clear;
> +
> + its_send_single_vcommand(find_4_1_its(), its_build_vsgi_cmd, &desc);
I see we pick up the first 4.1 ITS with find_4_1_its(). Can it happen
that not all of them have a mapping for that vPEID and if so we should
rather use one that has this mapping?
The spec says:
The ITS controls must only be used on an ITS that has a mapping for that
vPEID.
Where multiple ITSs have a mapping for the vPEID, any ITS with a mapping
may be used.
> +}
> +
> static int its_sgi_set_affinity(struct irq_data *d,
> const struct cpumask *mask_val,
> bool force)
> @@ -3915,13 +3984,21 @@ static void its_sgi_irq_domain_free(struct irq_domain *domain,
> static int its_sgi_irq_domain_activate(struct irq_domain *domain,
> struct irq_data *d, bool reserve)
> {
> + /* Write out the initial SGI configuration */
> + its_configure_sgi(d, false);
> return 0;
> }
>
> static void its_sgi_irq_domain_deactivate(struct irq_domain *domain,
> struct irq_data *d)
> {
> - /* Nothing to do */
> + struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
> +
> + /* First disable the SGI */
> + vpe->sgi_config[d->hwirq].enabled = false;
> + its_configure_sgi(d, false);
> + /* Now clear the potential pending bit (yes, this is clunky) */
nit: Without carefuly reading the VSGI cmd notes, it is difficult to
understand why those 2 steps are needed: maybe replace this comment by
something like:
to change the config, clear must be set to false. Then clear is set and
this leaves the config unchanged. Both steps cannot be done concurrently.
"
> + its_configure_sgi(d, true);
> }
>
> static struct irq_domain_ops its_sgi_domain_ops = {
> diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
> index b28acfa71f82..fd3be49ac9a5 100644
> --- a/include/linux/irqchip/arm-gic-v3.h
> +++ b/include/linux/irqchip/arm-gic-v3.h
> @@ -502,8 +502,9 @@
> #define GITS_CMD_VMAPTI GITS_CMD_GICv4(GITS_CMD_MAPTI)
> #define GITS_CMD_VMOVI GITS_CMD_GICv4(GITS_CMD_MOVI)
> #define GITS_CMD_VSYNC GITS_CMD_GICv4(GITS_CMD_SYNC)
> -/* VMOVP and INVDB are the odd ones, as they dont have a physical counterpart */
> +/* VMOVP, VSGI and INVDB are the odd ones, as they dont have a physical counterpart */
> #define GITS_CMD_VMOVP GITS_CMD_GICv4(2)
> +#define GITS_CMD_VSGI GITS_CMD_GICv4(3)
> #define GITS_CMD_INVDB GITS_CMD_GICv4(0xe)
>
> /*
>
Thanks
Eric
Hi Eric,
On 2020/3/17 1:53, Auger Eric wrote:
> Hi Marc,
>
> On 3/4/20 9:33 PM, Marc Zyngier wrote:
>> The GICv4.1 ITS has yet another new command (VSGI) which allows
>> a VPE-targeted SGI to be configured (or have its pending state
>> cleared). Add support for this command and plumb it into the
>> activate irqdomain callback so that it is ready to be used.
>>
>> Signed-off-by: Marc Zyngier <[email protected]>
>> Reviewed-by: Zenghui Yu <[email protected]>
>> ---
>> drivers/irqchip/irq-gic-v3-its.c | 79 +++++++++++++++++++++++++++++-
>> include/linux/irqchip/arm-gic-v3.h | 3 +-
>> 2 files changed, 80 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
>> index 112b452fcb40..e0db3f906f87 100644
>> --- a/drivers/irqchip/irq-gic-v3-its.c
>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>> @@ -380,6 +380,15 @@ struct its_cmd_desc {
>> struct {
>> struct its_vpe *vpe;
>> } its_invdb_cmd;
>> +
>> + struct {
>> + struct its_vpe *vpe;
>> + u8 sgi;
>> + u8 priority;
>> + bool enable;
>> + bool group;
>> + bool clear;
>> + } its_vsgi_cmd;
>> };
>> };
>>
>> @@ -528,6 +537,31 @@ static void its_encode_db(struct its_cmd_block *cmd, bool db)
>> its_mask_encode(&cmd->raw_cmd[2], db, 63, 63);
>> }
>>
>> +static void its_encode_sgi_intid(struct its_cmd_block *cmd, u8 sgi)
>> +{
>> + its_mask_encode(&cmd->raw_cmd[0], sgi, 35, 32);
>> +}
>> +
>> +static void its_encode_sgi_priority(struct its_cmd_block *cmd, u8 prio)
>> +{
>> + its_mask_encode(&cmd->raw_cmd[0], prio >> 4, 23, 20);
>> +}
>> +
>> +static void its_encode_sgi_group(struct its_cmd_block *cmd, bool grp)
>> +{
>> + its_mask_encode(&cmd->raw_cmd[0], grp, 10, 10);
>> +}
>> +
>> +static void its_encode_sgi_clear(struct its_cmd_block *cmd, bool clr)
>> +{
>> + its_mask_encode(&cmd->raw_cmd[0], clr, 9, 9);
>> +}
>> +
>> +static void its_encode_sgi_enable(struct its_cmd_block *cmd, bool en)
>> +{
>> + its_mask_encode(&cmd->raw_cmd[0], en, 8, 8);
>> +}
>> +
>> static inline void its_fixup_cmd(struct its_cmd_block *cmd)
>> {
>> /* Let's fixup BE commands */
>> @@ -893,6 +927,26 @@ static struct its_vpe *its_build_invdb_cmd(struct its_node *its,
>> return valid_vpe(its, desc->its_invdb_cmd.vpe);
>> }
>>
>> +static struct its_vpe *its_build_vsgi_cmd(struct its_node *its,
>> + struct its_cmd_block *cmd,
>> + struct its_cmd_desc *desc)
>> +{
>> + if (WARN_ON(!is_v4_1(its)))
>> + return NULL;
>> +
>> + its_encode_cmd(cmd, GITS_CMD_VSGI);
>> + its_encode_vpeid(cmd, desc->its_vsgi_cmd.vpe->vpe_id);
>> + its_encode_sgi_intid(cmd, desc->its_vsgi_cmd.sgi);
>> + its_encode_sgi_priority(cmd, desc->its_vsgi_cmd.priority);
>> + its_encode_sgi_group(cmd, desc->its_vsgi_cmd.group);
>> + its_encode_sgi_clear(cmd, desc->its_vsgi_cmd.clear);
>> + its_encode_sgi_enable(cmd, desc->its_vsgi_cmd.enable);
>> +
>> + its_fixup_cmd(cmd);
>> +
>> + return valid_vpe(its, desc->its_vsgi_cmd.vpe);
>> +}
>> +
>> static u64 its_cmd_ptr_to_offset(struct its_node *its,
>> struct its_cmd_block *ptr)
>> {
>> @@ -3870,6 +3924,21 @@ static struct irq_chip its_vpe_4_1_irq_chip = {
>> .irq_set_vcpu_affinity = its_vpe_4_1_set_vcpu_affinity,
>> };
>>
>> +static void its_configure_sgi(struct irq_data *d, bool clear)
>> +{
>> + struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
>> + struct its_cmd_desc desc;
>> +
>> + desc.its_vsgi_cmd.vpe = vpe;
>> + desc.its_vsgi_cmd.sgi = d->hwirq;
>> + desc.its_vsgi_cmd.priority = vpe->sgi_config[d->hwirq].priority;
>> + desc.its_vsgi_cmd.enable = vpe->sgi_config[d->hwirq].enabled;
>> + desc.its_vsgi_cmd.group = vpe->sgi_config[d->hwirq].group;
>> + desc.its_vsgi_cmd.clear = clear;
>> +
>> + its_send_single_vcommand(find_4_1_its(), its_build_vsgi_cmd, &desc);
> I see we pick up the first 4.1 ITS with find_4_1_its(). Can it happen
> that not all of them have a mapping for that vPEID and if so we should
> rather use one that has this mapping?
It can't happen in GICv4.1, and you may find the answer in patch #16
("Eagerly vmap vPEs"). I also failed to follow this logic the first
time looking at it [*], so I think it may worth adding some comments
on top of find_4_1_its()?
[*]
https://lore.kernel.org/lkml/[email protected]/
>
> The spec says:
> The ITS controls must only be used on an ITS that has a mapping for that
> vPEID.
> Where multiple ITSs have a mapping for the vPEID, any ITS with a mapping
> may be used.
>
>> +}
>> +
>> static int its_sgi_set_affinity(struct irq_data *d,
>> const struct cpumask *mask_val,
>> bool force)
>> @@ -3915,13 +3984,21 @@ static void its_sgi_irq_domain_free(struct irq_domain *domain,
>> static int its_sgi_irq_domain_activate(struct irq_domain *domain,
>> struct irq_data *d, bool reserve)
>> {
>> + /* Write out the initial SGI configuration */
>> + its_configure_sgi(d, false);
>> return 0;
>> }
>>
>> static void its_sgi_irq_domain_deactivate(struct irq_domain *domain,
>> struct irq_data *d)
>> {
>> - /* Nothing to do */
>> + struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
>> +
>> + /* First disable the SGI */
>> + vpe->sgi_config[d->hwirq].enabled = false;
>> + its_configure_sgi(d, false);
>> + /* Now clear the potential pending bit (yes, this is clunky) */
> nit: Without carefuly reading the VSGI cmd notes, it is difficult to
> understand why those 2 steps are needed: maybe replace this comment by
> something like:
> to change the config, clear must be set to false. Then clear is set and
> this leaves the config unchanged. Both steps cannot be done concurrently.
>
> "
I think it makes sense.
Thanks,
Zenghui
>> + its_configure_sgi(d, true);
>> }
>>
>> static struct irq_domain_ops its_sgi_domain_ops = {
>> diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
>> index b28acfa71f82..fd3be49ac9a5 100644
>> --- a/include/linux/irqchip/arm-gic-v3.h
>> +++ b/include/linux/irqchip/arm-gic-v3.h
>> @@ -502,8 +502,9 @@
>> #define GITS_CMD_VMAPTI GITS_CMD_GICv4(GITS_CMD_MAPTI)
>> #define GITS_CMD_VMOVI GITS_CMD_GICv4(GITS_CMD_MOVI)
>> #define GITS_CMD_VSYNC GITS_CMD_GICv4(GITS_CMD_SYNC)
>> -/* VMOVP and INVDB are the odd ones, as they dont have a physical counterpart */
>> +/* VMOVP, VSGI and INVDB are the odd ones, as they dont have a physical counterpart */
>> #define GITS_CMD_VMOVP GITS_CMD_GICv4(2)
>> +#define GITS_CMD_VSGI GITS_CMD_GICv4(3)
>> #define GITS_CMD_INVDB GITS_CMD_GICv4(0xe)
>>
>> /*
>>
> Thanks
>
> Eric
>
>
> .
>
Hi Zenghui,
On 3/17/20 3:02 AM, Zenghui Yu wrote:
> Hi Eric,
>
> On 2020/3/17 1:53, Auger Eric wrote:
>> Hi Marc,
>>
>> On 3/4/20 9:33 PM, Marc Zyngier wrote:
>>> The GICv4.1 ITS has yet another new command (VSGI) which allows
>>> a VPE-targeted SGI to be configured (or have its pending state
>>> cleared). Add support for this command and plumb it into the
>>> activate irqdomain callback so that it is ready to be used.
>>>
>>> Signed-off-by: Marc Zyngier <[email protected]>
>>> Reviewed-by: Zenghui Yu <[email protected]>
>>> ---
>>> ? drivers/irqchip/irq-gic-v3-its.c?? | 79 +++++++++++++++++++++++++++++-
>>> ? include/linux/irqchip/arm-gic-v3.h |? 3 +-
>>> ? 2 files changed, 80 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/irqchip/irq-gic-v3-its.c
>>> b/drivers/irqchip/irq-gic-v3-its.c
>>> index 112b452fcb40..e0db3f906f87 100644
>>> --- a/drivers/irqchip/irq-gic-v3-its.c
>>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>>> @@ -380,6 +380,15 @@ struct its_cmd_desc {
>>> ????????? struct {
>>> ????????????? struct its_vpe *vpe;
>>> ????????? } its_invdb_cmd;
>>> +
>>> +??????? struct {
>>> +??????????? struct its_vpe *vpe;
>>> +??????????? u8 sgi;
>>> +??????????? u8 priority;
>>> +??????????? bool enable;
>>> +??????????? bool group;
>>> +??????????? bool clear;
>>> +??????? } its_vsgi_cmd;
>>> ????? };
>>> ? };
>>> ? @@ -528,6 +537,31 @@ static void its_encode_db(struct its_cmd_block
>>> *cmd, bool db)
>>> ????? its_mask_encode(&cmd->raw_cmd[2], db, 63, 63);
>>> ? }
>>> ? +static void its_encode_sgi_intid(struct its_cmd_block *cmd, u8 sgi)
>>> +{
>>> +??? its_mask_encode(&cmd->raw_cmd[0], sgi, 35, 32);
>>> +}
>>> +
>>> +static void its_encode_sgi_priority(struct its_cmd_block *cmd, u8 prio)
>>> +{
>>> +??? its_mask_encode(&cmd->raw_cmd[0], prio >> 4, 23, 20);
>>> +}
>>> +
>>> +static void its_encode_sgi_group(struct its_cmd_block *cmd, bool grp)
>>> +{
>>> +??? its_mask_encode(&cmd->raw_cmd[0], grp, 10, 10);
>>> +}
>>> +
>>> +static void its_encode_sgi_clear(struct its_cmd_block *cmd, bool clr)
>>> +{
>>> +??? its_mask_encode(&cmd->raw_cmd[0], clr, 9, 9);
>>> +}
>>> +
>>> +static void its_encode_sgi_enable(struct its_cmd_block *cmd, bool en)
>>> +{
>>> +??? its_mask_encode(&cmd->raw_cmd[0], en, 8, 8);
>>> +}
>>> +
>>> ? static inline void its_fixup_cmd(struct its_cmd_block *cmd)
>>> ? {
>>> ????? /* Let's fixup BE commands */
>>> @@ -893,6 +927,26 @@ static struct its_vpe
>>> *its_build_invdb_cmd(struct its_node *its,
>>> ????? return valid_vpe(its, desc->its_invdb_cmd.vpe);
>>> ? }
>>> ? +static struct its_vpe *its_build_vsgi_cmd(struct its_node *its,
>>> +????????????????????? struct its_cmd_block *cmd,
>>> +????????????????????? struct its_cmd_desc *desc)
>>> +{
>>> +??? if (WARN_ON(!is_v4_1(its)))
>>> +??????? return NULL;
>>> +
>>> +??? its_encode_cmd(cmd, GITS_CMD_VSGI);
>>> +??? its_encode_vpeid(cmd, desc->its_vsgi_cmd.vpe->vpe_id);
>>> +??? its_encode_sgi_intid(cmd, desc->its_vsgi_cmd.sgi);
>>> +??? its_encode_sgi_priority(cmd, desc->its_vsgi_cmd.priority);
>>> +??? its_encode_sgi_group(cmd, desc->its_vsgi_cmd.group);
>>> +??? its_encode_sgi_clear(cmd, desc->its_vsgi_cmd.clear);
>>> +??? its_encode_sgi_enable(cmd, desc->its_vsgi_cmd.enable);
>>> +
>>> +??? its_fixup_cmd(cmd);
>>> +
>>> +??? return valid_vpe(its, desc->its_vsgi_cmd.vpe);
>>> +}
>>> +
>>> ? static u64 its_cmd_ptr_to_offset(struct its_node *its,
>>> ?????????????????? struct its_cmd_block *ptr)
>>> ? {
>>> @@ -3870,6 +3924,21 @@ static struct irq_chip its_vpe_4_1_irq_chip = {
>>> ????? .irq_set_vcpu_affinity??? = its_vpe_4_1_set_vcpu_affinity,
>>> ? };
>>> ? +static void its_configure_sgi(struct irq_data *d, bool clear)
>>> +{
>>> +??? struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
>>> +??? struct its_cmd_desc desc;
>>> +
>>> +??? desc.its_vsgi_cmd.vpe = vpe;
>>> +??? desc.its_vsgi_cmd.sgi = d->hwirq;
>>> +??? desc.its_vsgi_cmd.priority = vpe->sgi_config[d->hwirq].priority;
>>> +??? desc.its_vsgi_cmd.enable = vpe->sgi_config[d->hwirq].enabled;
>>> +??? desc.its_vsgi_cmd.group = vpe->sgi_config[d->hwirq].group;
>>> +??? desc.its_vsgi_cmd.clear = clear;
>>> +
>>> +??? its_send_single_vcommand(find_4_1_its(), its_build_vsgi_cmd,
>>> &desc);
>> I see we pick up the first 4.1 ITS with find_4_1_its(). Can it happen
>> that not all of them have a mapping for that vPEID and if so we should
>> rather use one that has this mapping?
>
> It can't happen in GICv4.1, and you may find the answer in patch #16
> ("Eagerly vmap vPEs").? I also failed to follow this logic the first
> time looking at it [*], so I think it may worth adding some comments
> on top of find_4_1_its()?
>
> [*]
> https://lore.kernel.org/lkml/[email protected]/
OK thank you for the pointer. Slowly moving forward ... ;-)
Eric
>
>
>>
>> The spec says:
>> The ITS controls must only be used on an ITS that has a mapping for that
>> vPEID.
>> Where multiple ITSs have a mapping for the vPEID, any ITS with a mapping
>> may be used.
>>
>>> +}
>>> +
>>> ? static int its_sgi_set_affinity(struct irq_data *d,
>>> ????????????????? const struct cpumask *mask_val,
>>> ????????????????? bool force)
>>> @@ -3915,13 +3984,21 @@ static void its_sgi_irq_domain_free(struct
>>> irq_domain *domain,
>>> ? static int its_sgi_irq_domain_activate(struct irq_domain *domain,
>>> ???????????????????????? struct irq_data *d, bool reserve)
>>> ? {
>>> +??? /* Write out the initial SGI configuration */
>>> +??? its_configure_sgi(d, false);
>>> ????? return 0;
>>> ? }
>>> ? ? static void its_sgi_irq_domain_deactivate(struct irq_domain *domain,
>>> ??????????????????????? struct irq_data *d)
>>> ? {
>>> -??? /* Nothing to do */
>>> +??? struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
>>> +
>>> +??? /* First disable the SGI */
>>> +??? vpe->sgi_config[d->hwirq].enabled = false;
>>> +??? its_configure_sgi(d, false);
>>> +??? /* Now clear the potential pending bit (yes, this is clunky) */
>> nit: Without carefuly reading the VSGI cmd notes, it is difficult to
>> understand why those 2 steps are needed: maybe replace this comment by
>> something like:
>> to change the config, clear must be set to false. Then clear is set and
>> this leaves the config unchanged. Both steps cannot be done concurrently.
>>
>> "
>
> I think it makes sense.
>
>
> Thanks,
> Zenghui
>
>>> +??? its_configure_sgi(d, true);
>>> ? }
>>> ? ? static struct irq_domain_ops its_sgi_domain_ops = {
>>> diff --git a/include/linux/irqchip/arm-gic-v3.h
>>> b/include/linux/irqchip/arm-gic-v3.h
>>> index b28acfa71f82..fd3be49ac9a5 100644
>>> --- a/include/linux/irqchip/arm-gic-v3.h
>>> +++ b/include/linux/irqchip/arm-gic-v3.h
>>> @@ -502,8 +502,9 @@
>>> ? #define GITS_CMD_VMAPTI??????????? GITS_CMD_GICv4(GITS_CMD_MAPTI)
>>> ? #define GITS_CMD_VMOVI??????????? GITS_CMD_GICv4(GITS_CMD_MOVI)
>>> ? #define GITS_CMD_VSYNC??????????? GITS_CMD_GICv4(GITS_CMD_SYNC)
>>> -/* VMOVP and INVDB are the odd ones, as they dont have a physical
>>> counterpart */
>>> +/* VMOVP, VSGI and INVDB are the odd ones, as they dont have a
>>> physical counterpart */
>>> ? #define GITS_CMD_VMOVP??????????? GITS_CMD_GICv4(2)
>>> +#define GITS_CMD_VSGI??????????? GITS_CMD_GICv4(3)
>>> ? #define GITS_CMD_INVDB??????????? GITS_CMD_GICv4(0xe)
>>> ? ? /*
>>>
>> Thanks
>>
>> Eric
>>
>>
>> .
>>
>
Hi Eric,
On 2020-03-16 17:53, Auger Eric wrote:
> Hi Marc,
>
> On 3/4/20 9:33 PM, Marc Zyngier wrote:
>> The GICv4.1 ITS has yet another new command (VSGI) which allows
>> a VPE-targeted SGI to be configured (or have its pending state
>> cleared). Add support for this command and plumb it into the
>> activate irqdomain callback so that it is ready to be used.
>>
>> Signed-off-by: Marc Zyngier <[email protected]>
>> Reviewed-by: Zenghui Yu <[email protected]>
>> ---
>> drivers/irqchip/irq-gic-v3-its.c | 79
>> +++++++++++++++++++++++++++++-
>> include/linux/irqchip/arm-gic-v3.h | 3 +-
>> 2 files changed, 80 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-gic-v3-its.c
>> b/drivers/irqchip/irq-gic-v3-its.c
>> index 112b452fcb40..e0db3f906f87 100644
>> --- a/drivers/irqchip/irq-gic-v3-its.c
>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>> @@ -380,6 +380,15 @@ struct its_cmd_desc {
>> struct {
>> struct its_vpe *vpe;
>> } its_invdb_cmd;
>> +
>> + struct {
>> + struct its_vpe *vpe;
>> + u8 sgi;
>> + u8 priority;
>> + bool enable;
>> + bool group;
>> + bool clear;
>> + } its_vsgi_cmd;
>> };
>> };
>>
>> @@ -528,6 +537,31 @@ static void its_encode_db(struct its_cmd_block
>> *cmd, bool db)
>> its_mask_encode(&cmd->raw_cmd[2], db, 63, 63);
>> }
>>
>> +static void its_encode_sgi_intid(struct its_cmd_block *cmd, u8 sgi)
>> +{
>> + its_mask_encode(&cmd->raw_cmd[0], sgi, 35, 32);
>> +}
>> +
>> +static void its_encode_sgi_priority(struct its_cmd_block *cmd, u8
>> prio)
>> +{
>> + its_mask_encode(&cmd->raw_cmd[0], prio >> 4, 23, 20);
>> +}
>> +
>> +static void its_encode_sgi_group(struct its_cmd_block *cmd, bool grp)
>> +{
>> + its_mask_encode(&cmd->raw_cmd[0], grp, 10, 10);
>> +}
>> +
>> +static void its_encode_sgi_clear(struct its_cmd_block *cmd, bool clr)
>> +{
>> + its_mask_encode(&cmd->raw_cmd[0], clr, 9, 9);
>> +}
>> +
>> +static void its_encode_sgi_enable(struct its_cmd_block *cmd, bool en)
>> +{
>> + its_mask_encode(&cmd->raw_cmd[0], en, 8, 8);
>> +}
>> +
>> static inline void its_fixup_cmd(struct its_cmd_block *cmd)
>> {
>> /* Let's fixup BE commands */
>> @@ -893,6 +927,26 @@ static struct its_vpe *its_build_invdb_cmd(struct
>> its_node *its,
>> return valid_vpe(its, desc->its_invdb_cmd.vpe);
>> }
>>
>> +static struct its_vpe *its_build_vsgi_cmd(struct its_node *its,
>> + struct its_cmd_block *cmd,
>> + struct its_cmd_desc *desc)
>> +{
>> + if (WARN_ON(!is_v4_1(its)))
>> + return NULL;
>> +
>> + its_encode_cmd(cmd, GITS_CMD_VSGI);
>> + its_encode_vpeid(cmd, desc->its_vsgi_cmd.vpe->vpe_id);
>> + its_encode_sgi_intid(cmd, desc->its_vsgi_cmd.sgi);
>> + its_encode_sgi_priority(cmd, desc->its_vsgi_cmd.priority);
>> + its_encode_sgi_group(cmd, desc->its_vsgi_cmd.group);
>> + its_encode_sgi_clear(cmd, desc->its_vsgi_cmd.clear);
>> + its_encode_sgi_enable(cmd, desc->its_vsgi_cmd.enable);
>> +
>> + its_fixup_cmd(cmd);
>> +
>> + return valid_vpe(its, desc->its_vsgi_cmd.vpe);
>> +}
>> +
>> static u64 its_cmd_ptr_to_offset(struct its_node *its,
>> struct its_cmd_block *ptr)
>> {
>> @@ -3870,6 +3924,21 @@ static struct irq_chip its_vpe_4_1_irq_chip = {
>> .irq_set_vcpu_affinity = its_vpe_4_1_set_vcpu_affinity,
>> };
>>
>> +static void its_configure_sgi(struct irq_data *d, bool clear)
>> +{
>> + struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
>> + struct its_cmd_desc desc;
>> +
>> + desc.its_vsgi_cmd.vpe = vpe;
>> + desc.its_vsgi_cmd.sgi = d->hwirq;
>> + desc.its_vsgi_cmd.priority = vpe->sgi_config[d->hwirq].priority;
>> + desc.its_vsgi_cmd.enable = vpe->sgi_config[d->hwirq].enabled;
>> + desc.its_vsgi_cmd.group = vpe->sgi_config[d->hwirq].group;
>> + desc.its_vsgi_cmd.clear = clear;
>> +
>> + its_send_single_vcommand(find_4_1_its(), its_build_vsgi_cmd, &desc);
> I see we pick up the first 4.1 ITS with find_4_1_its(). Can it happen
> that not all of them have a mapping for that vPEID and if so we should
> rather use one that has this mapping?
>
> The spec says:
> The ITS controls must only be used on an ITS that has a mapping for
> that
> vPEID.
> Where multiple ITSs have a mapping for the vPEID, any ITS with a
> mapping
> may be used.
As Zenghui pointed out, we eagerly map all the VPEs, as we need the
vSGIs
to be delivered by the ITS (yes, this is pretty backward, both in the
series
and the architecture).
I'll add this coment to lift the ambiguity:
/*
* GICv4.1 allows us to send VSGI commands to any ITS as long as the
* destination VPE is mapped there. Since we map them eagerly at
* activation time, we're pretty sure the first GICv4.1 ITS will do.
*/
>
>> +}
>> +
>> static int its_sgi_set_affinity(struct irq_data *d,
>> const struct cpumask *mask_val,
>> bool force)
>> @@ -3915,13 +3984,21 @@ static void its_sgi_irq_domain_free(struct
>> irq_domain *domain,
>> static int its_sgi_irq_domain_activate(struct irq_domain *domain,
>> struct irq_data *d, bool reserve)
>> {
>> + /* Write out the initial SGI configuration */
>> + its_configure_sgi(d, false);
>> return 0;
>> }
>>
>> static void its_sgi_irq_domain_deactivate(struct irq_domain *domain,
>> struct irq_data *d)
>> {
>> - /* Nothing to do */
>> + struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
>> +
>> + /* First disable the SGI */
>> + vpe->sgi_config[d->hwirq].enabled = false;
>> + its_configure_sgi(d, false);
>> + /* Now clear the potential pending bit (yes, this is clunky) */
> nit: Without carefuly reading the VSGI cmd notes, it is difficult to
> understand why those 2 steps are needed: maybe replace this comment by
> something like:
> to change the config, clear must be set to false. Then clear is set and
> this leaves the config unchanged. Both steps cannot be done
> concurrently.
I've added this:
/*
* The VSGI command is awkward:
*
* - To change the configuration, CLEAR must be set to false,
* leaving the pending bit unchanged.
* - To clear the pending bit, CLEAR must be set to true, leaving
* the configuration unchanged.
*
* You just can't do both at once, hence the two commands below.
*/
Thanks,
M.
--
Jazz is not dead. It just smells funny...
The following commit has been merged into the irq/core branch of tip:
Commit-ID: e252cf8a34d92adf41124cb59b19b49d25395548
Gitweb: https://git.kernel.org/tip/e252cf8a34d92adf41124cb59b19b49d25395548
Author: Marc Zyngier <[email protected]>
AuthorDate: Wed, 04 Mar 2020 20:33:16
Committer: Marc Zyngier <[email protected]>
CommitterDate: Tue, 24 Mar 2020 12:05:08
irqchip/gic-v4.1: Add initial SGI configuration
The GICv4.1 ITS has yet another new command (VSGI) which allows
a VPE-targeted SGI to be configured (or have its pending state
cleared). Add support for this command and plumb it into the
activate irqdomain callback so that it is ready to be used.
Signed-off-by: Marc Zyngier <[email protected]>
Reviewed-by: Zenghui Yu <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/irqchip/irq-gic-v3-its.c | 92 ++++++++++++++++++++++++++++-
include/linux/irqchip/arm-gic-v3.h | 3 +-
2 files changed, 93 insertions(+), 2 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index c9c8121..28c856a 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -380,6 +380,15 @@ struct its_cmd_desc {
struct {
struct its_vpe *vpe;
} its_invdb_cmd;
+
+ struct {
+ struct its_vpe *vpe;
+ u8 sgi;
+ u8 priority;
+ bool enable;
+ bool group;
+ bool clear;
+ } its_vsgi_cmd;
};
};
@@ -528,6 +537,31 @@ static void its_encode_db(struct its_cmd_block *cmd, bool db)
its_mask_encode(&cmd->raw_cmd[2], db, 63, 63);
}
+static void its_encode_sgi_intid(struct its_cmd_block *cmd, u8 sgi)
+{
+ its_mask_encode(&cmd->raw_cmd[0], sgi, 35, 32);
+}
+
+static void its_encode_sgi_priority(struct its_cmd_block *cmd, u8 prio)
+{
+ its_mask_encode(&cmd->raw_cmd[0], prio >> 4, 23, 20);
+}
+
+static void its_encode_sgi_group(struct its_cmd_block *cmd, bool grp)
+{
+ its_mask_encode(&cmd->raw_cmd[0], grp, 10, 10);
+}
+
+static void its_encode_sgi_clear(struct its_cmd_block *cmd, bool clr)
+{
+ its_mask_encode(&cmd->raw_cmd[0], clr, 9, 9);
+}
+
+static void its_encode_sgi_enable(struct its_cmd_block *cmd, bool en)
+{
+ its_mask_encode(&cmd->raw_cmd[0], en, 8, 8);
+}
+
static inline void its_fixup_cmd(struct its_cmd_block *cmd)
{
/* Let's fixup BE commands */
@@ -893,6 +927,26 @@ static struct its_vpe *its_build_invdb_cmd(struct its_node *its,
return valid_vpe(its, desc->its_invdb_cmd.vpe);
}
+static struct its_vpe *its_build_vsgi_cmd(struct its_node *its,
+ struct its_cmd_block *cmd,
+ struct its_cmd_desc *desc)
+{
+ if (WARN_ON(!is_v4_1(its)))
+ return NULL;
+
+ its_encode_cmd(cmd, GITS_CMD_VSGI);
+ its_encode_vpeid(cmd, desc->its_vsgi_cmd.vpe->vpe_id);
+ its_encode_sgi_intid(cmd, desc->its_vsgi_cmd.sgi);
+ its_encode_sgi_priority(cmd, desc->its_vsgi_cmd.priority);
+ its_encode_sgi_group(cmd, desc->its_vsgi_cmd.group);
+ its_encode_sgi_clear(cmd, desc->its_vsgi_cmd.clear);
+ its_encode_sgi_enable(cmd, desc->its_vsgi_cmd.enable);
+
+ its_fixup_cmd(cmd);
+
+ return valid_vpe(its, desc->its_vsgi_cmd.vpe);
+}
+
static u64 its_cmd_ptr_to_offset(struct its_node *its,
struct its_cmd_block *ptr)
{
@@ -3870,6 +3924,26 @@ static struct irq_chip its_vpe_4_1_irq_chip = {
.irq_set_vcpu_affinity = its_vpe_4_1_set_vcpu_affinity,
};
+static void its_configure_sgi(struct irq_data *d, bool clear)
+{
+ struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
+ struct its_cmd_desc desc;
+
+ desc.its_vsgi_cmd.vpe = vpe;
+ desc.its_vsgi_cmd.sgi = d->hwirq;
+ desc.its_vsgi_cmd.priority = vpe->sgi_config[d->hwirq].priority;
+ desc.its_vsgi_cmd.enable = vpe->sgi_config[d->hwirq].enabled;
+ desc.its_vsgi_cmd.group = vpe->sgi_config[d->hwirq].group;
+ desc.its_vsgi_cmd.clear = clear;
+
+ /*
+ * GICv4.1 allows us to send VSGI commands to any ITS as long as the
+ * destination VPE is mapped there. Since we map them eagerly at
+ * activation time, we're pretty sure the first GICv4.1 ITS will do.
+ */
+ its_send_single_vcommand(find_4_1_its(), its_build_vsgi_cmd, &desc);
+}
+
static int its_sgi_set_affinity(struct irq_data *d,
const struct cpumask *mask_val,
bool force)
@@ -3920,13 +3994,29 @@ static void its_sgi_irq_domain_free(struct irq_domain *domain,
static int its_sgi_irq_domain_activate(struct irq_domain *domain,
struct irq_data *d, bool reserve)
{
+ /* Write out the initial SGI configuration */
+ its_configure_sgi(d, false);
return 0;
}
static void its_sgi_irq_domain_deactivate(struct irq_domain *domain,
struct irq_data *d)
{
- /* Nothing to do */
+ struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
+
+ /*
+ * The VSGI command is awkward:
+ *
+ * - To change the configuration, CLEAR must be set to false,
+ * leaving the pending bit unchanged.
+ * - To clear the pending bit, CLEAR must be set to true, leaving
+ * the configuration unchanged.
+ *
+ * You just can't do both at once, hence the two commands below.
+ */
+ vpe->sgi_config[d->hwirq].enabled = false;
+ its_configure_sgi(d, false);
+ its_configure_sgi(d, true);
}
static const struct irq_domain_ops its_sgi_domain_ops = {
diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
index b28acfa..fd3be49 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -502,8 +502,9 @@
#define GITS_CMD_VMAPTI GITS_CMD_GICv4(GITS_CMD_MAPTI)
#define GITS_CMD_VMOVI GITS_CMD_GICv4(GITS_CMD_MOVI)
#define GITS_CMD_VSYNC GITS_CMD_GICv4(GITS_CMD_SYNC)
-/* VMOVP and INVDB are the odd ones, as they dont have a physical counterpart */
+/* VMOVP, VSGI and INVDB are the odd ones, as they dont have a physical counterpart */
#define GITS_CMD_VMOVP GITS_CMD_GICv4(2)
+#define GITS_CMD_VSGI GITS_CMD_GICv4(3)
#define GITS_CMD_INVDB GITS_CMD_GICv4(0xe)
/*