* [RFC 0/2] irqchip/gic-v3-its: Introduce virtual ITS
@ 2021-06-22 15:53 ` Boqun Feng
0 siblings, 0 replies; 12+ messages in thread
From: Boqun Feng @ 2021-06-22 15:53 UTC (permalink / raw)
To: Marc Zyngier
Cc: Thomas Gleixner, linux-kernel, Arnd Bergmann, Bjorn Helgaas,
Linux ARM, Catalin Marinas, Will Deacon, Boqun Feng
Hi Marc,
Here is an RFC for supporting platforms having LPI supported but without
ITS. And this is for the virtual PCI support for ARM64 Hyper-V guests.
We currently choose this approach (LPI w/o ITS) because a) it's allowed
for GICv3 and b) ITS may not be a more efficient way to configure LPIs
compared to hypercalls, but we'd like to get feedbacks from the
community.
Besides, patch #1 fixes a bug which I found while I was at it.
Looking forwards to any comment and suggestion!
Regards,
Boqun
Boqun Feng (2):
irqchip/gic-v3-its: Free collections if its domain initialization
fails
irqchip/gic-v3-its: Introduce virtual ITS
drivers/irqchip/irq-gic-v3-its.c | 124 ++++++++++++++++++++++++++++---
1 file changed, 115 insertions(+), 9 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 12+ messages in thread* [RFC 0/2] irqchip/gic-v3-its: Introduce virtual ITS @ 2021-06-22 15:53 ` Boqun Feng 0 siblings, 0 replies; 12+ messages in thread From: Boqun Feng @ 2021-06-22 15:53 UTC (permalink / raw) To: Marc Zyngier Cc: Thomas Gleixner, linux-kernel, Arnd Bergmann, Bjorn Helgaas, Linux ARM, Catalin Marinas, Will Deacon, Boqun Feng Hi Marc, Here is an RFC for supporting platforms having LPI supported but without ITS. And this is for the virtual PCI support for ARM64 Hyper-V guests. We currently choose this approach (LPI w/o ITS) because a) it's allowed for GICv3 and b) ITS may not be a more efficient way to configure LPIs compared to hypercalls, but we'd like to get feedbacks from the community. Besides, patch #1 fixes a bug which I found while I was at it. Looking forwards to any comment and suggestion! Regards, Boqun Boqun Feng (2): irqchip/gic-v3-its: Free collections if its domain initialization fails irqchip/gic-v3-its: Introduce virtual ITS drivers/irqchip/irq-gic-v3-its.c | 124 ++++++++++++++++++++++++++++--- 1 file changed, 115 insertions(+), 9 deletions(-) -- 2.30.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFC 1/2] irqchip/gic-v3-its: Free collections if its domain initialization fails 2021-06-22 15:53 ` Boqun Feng @ 2021-06-22 15:53 ` Boqun Feng -1 siblings, 0 replies; 12+ messages in thread From: Boqun Feng @ 2021-06-22 15:53 UTC (permalink / raw) To: Marc Zyngier Cc: Thomas Gleixner, linux-kernel, Arnd Bergmann, Bjorn Helgaas, Linux ARM, Catalin Marinas, Will Deacon, Boqun Feng ITS collections are allocated before its_init_domain() called in its_init(), therefore if its_init_domain() fails, the collections need to be freed. This fixes a potential memory leak. Signed-off-by: Boqun Feng <boqun.feng@gmail.com> --- drivers/irqchip/irq-gic-v3-its.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c index 2e6923c2c8a8..1916ac5d6371 100644 --- a/drivers/irqchip/irq-gic-v3-its.c +++ b/drivers/irqchip/irq-gic-v3-its.c @@ -2931,6 +2931,12 @@ static int its_alloc_collections(struct its_node *its) return 0; } +static void its_free_collections(struct its_node *its) +{ + if (its) + kfree(its->collections); +} + static struct page *its_allocate_pending_table(gfp_t gfp_flags) { struct page *pend_page; @@ -5090,7 +5096,7 @@ static int __init its_probe_one(struct resource *res, err = its_init_domain(handle, its); if (err) - goto out_free_tables; + goto out_free_collections; raw_spin_lock(&its_lock); list_add(&its->entry, &its_nodes); @@ -5098,6 +5104,8 @@ static int __init its_probe_one(struct resource *res, return 0; +out_free_collections: + its_free_collections(its); out_free_tables: its_free_tables(its); out_free_cmd: -- 2.30.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [RFC 1/2] irqchip/gic-v3-its: Free collections if its domain initialization fails @ 2021-06-22 15:53 ` Boqun Feng 0 siblings, 0 replies; 12+ messages in thread From: Boqun Feng @ 2021-06-22 15:53 UTC (permalink / raw) To: Marc Zyngier Cc: Thomas Gleixner, linux-kernel, Arnd Bergmann, Bjorn Helgaas, Linux ARM, Catalin Marinas, Will Deacon, Boqun Feng ITS collections are allocated before its_init_domain() called in its_init(), therefore if its_init_domain() fails, the collections need to be freed. This fixes a potential memory leak. Signed-off-by: Boqun Feng <boqun.feng@gmail.com> --- drivers/irqchip/irq-gic-v3-its.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c index 2e6923c2c8a8..1916ac5d6371 100644 --- a/drivers/irqchip/irq-gic-v3-its.c +++ b/drivers/irqchip/irq-gic-v3-its.c @@ -2931,6 +2931,12 @@ static int its_alloc_collections(struct its_node *its) return 0; } +static void its_free_collections(struct its_node *its) +{ + if (its) + kfree(its->collections); +} + static struct page *its_allocate_pending_table(gfp_t gfp_flags) { struct page *pend_page; @@ -5090,7 +5096,7 @@ static int __init its_probe_one(struct resource *res, err = its_init_domain(handle, its); if (err) - goto out_free_tables; + goto out_free_collections; raw_spin_lock(&its_lock); list_add(&its->entry, &its_nodes); @@ -5098,6 +5104,8 @@ static int __init its_probe_one(struct resource *res, return 0; +out_free_collections: + its_free_collections(its); out_free_tables: its_free_tables(its); out_free_cmd: -- 2.30.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [RFC 2/2] irqchip/gic-v3-its: Introduce virtual ITS 2021-06-22 15:53 ` Boqun Feng @ 2021-06-22 15:53 ` Boqun Feng -1 siblings, 0 replies; 12+ messages in thread From: Boqun Feng @ 2021-06-22 15:53 UTC (permalink / raw) To: Marc Zyngier Cc: Thomas Gleixner, linux-kernel, Arnd Bergmann, Bjorn Helgaas, Linux ARM, Catalin Marinas, Will Deacon, Boqun Feng, Sunil Muthuswamy GICv3 allows supporting LPI without an ITS, and in order to support such a platform, a virtual ITS is introduced. The virtual ITS has the same software part as a real ITS: having an irq domain, maintaining ->collections and maintaining the list of devices. The only difference is the virtual ITS doesn't have a backed ITS therefore it cannot issue ITS commands nor set up device tables. The virtual ITS only manages LPIs and the LPIs are configured via DirectLPI interfaces. And currently the virtual ITS is initialized only if there is no ITS in the system and yet DirectLPI is support. The virtual ITS approach provides the support for LPI without an ITS and reuses as much exisiting code as possible, and is the preparation for virtual PCI support on ARM64 Hyper-V guests. Co-developed-by: Sunil Muthuswamy <sunilmut@microsoft.com> Signed-off-by: Boqun Feng <boqun.feng@gmail.com> --- drivers/irqchip/irq-gic-v3-its.c | 114 ++++++++++++++++++++++++++++--- 1 file changed, 106 insertions(+), 8 deletions(-) diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c index 1916ac5d6371..4f2600377039 100644 --- a/drivers/irqchip/irq-gic-v3-its.c +++ b/drivers/irqchip/irq-gic-v3-its.c @@ -117,9 +117,16 @@ struct its_node { int vlpi_redist_offset; }; +/* + * LPI can be supported without ITS, in which case, a virtual its_node is + * initialized to allow configuring LPI with the DirectLPI approach. + */ +static struct its_node *virtual_its_node; + #define is_v4(its) (!!((its)->typer & GITS_TYPER_VLPIS)) #define is_v4_1(its) (!!((its)->typer & GITS_TYPER_VMAPP)) #define device_ids(its) (FIELD_GET(GITS_TYPER_DEVBITS, (its)->typer) + 1) +#define is_virtual(its) ((its) == virtual_its_node) #define ITS_ITT_ALIGN SZ_256 @@ -1096,6 +1103,10 @@ void name(struct its_node *its, \ unsigned long flags; \ u64 rd_idx; \ \ + /* Virtual ITS doesn't support ITS commands */ \ + if (is_virtual(its)) \ + return; \ + \ raw_spin_lock_irqsave(&its->lock, flags); \ \ cmd = its_allocate_entry(its); \ @@ -1464,7 +1475,8 @@ static void lpi_update_config(struct irq_data *d, u8 clr, u8 set) lpi_write_config(d, clr, set); if (gic_rdists->has_direct_lpi && - (is_v4_1(its_dev->its) || !irqd_is_forwarded_to_vcpu(d))) + (is_v4_1(its_dev->its) || !irqd_is_forwarded_to_vcpu(d) || + is_virtual(its_dev->its))) direct_lpi_inv(d); else if (!irqd_is_forwarded_to_vcpu(d)) its_send_inv(its_dev, its_get_event_id(d)); @@ -1690,6 +1702,10 @@ static void its_irq_compose_msi_msg(struct irq_data *d, struct msi_msg *msg) u64 addr; its = its_dev->its; + + /* Virtual ITS doesn't have ->get_msi_base function, skip */ + if (!its->get_msi_base) + return; addr = its->get_msi_base(its_dev); msg->address_lo = lower_32_bits(addr); @@ -3217,7 +3233,17 @@ static void its_cpu_init_collections(void) raw_spin_lock(&its_lock); list_for_each_entry(its, &its_nodes, entry) - its_cpu_init_collection(its); + /* + * Only initializes the software part of collections for virtual + * ITS. + */ + if (is_virtual(its)) { + int cpu = smp_processor_id(); + + its->collections[cpu].col_id = cpu; + } else { + its_cpu_init_collection(its); + } raw_spin_unlock(&its_lock); } @@ -3364,7 +3390,8 @@ static struct its_device *its_create_device(struct its_node *its, u32 dev_id, int nr_ites; int sz; - if (!its_alloc_device_table(its, dev_id)) + /* No need to allocate device table for virtual ITS */ + if (!is_virtual(its) && !its_alloc_device_table(its, dev_id)) return NULL; if (WARN_ON(!is_power_of_2(nvecs))) @@ -3551,9 +3578,12 @@ static int its_irq_domain_alloc(struct irq_domain *domain, unsigned int virq, if (err) return err; - err = iommu_dma_prepare_msi(info->desc, its->get_msi_base(its_dev)); - if (err) - return err; + /* Virtual ITS doesn't have ->get_msi_base function, skip */ + if (its->get_msi_base) { + err = iommu_dma_prepare_msi(info->desc, its->get_msi_base(its_dev)); + if (err) + return err; + } for (i = 0; i < nr_irqs; i++) { err = its_irq_gic_domain_alloc(domain, virq + i, hwirq + i); @@ -5121,6 +5151,63 @@ static int __init its_probe_one(struct resource *res, return err; } +static int __init virtual_its_init(void) +{ + struct its_node *its; + struct fwnode_handle *fwnode; + int err; + + fwnode = irq_domain_alloc_named_fwnode("Virtual ITS"); + if (!fwnode) + return -ENOMEM; + + /* + * Use 0 as the ID for virtual ITS, since we only init virtual ITS if + * there is no real ITS in the system, so it's fine. + */ + err = iort_register_domain_token(0, 0, fwnode); + if (err) + goto out_free_fwnode; + + its = kzalloc(sizeof(*its), GFP_KERNEL); + if (!its) { + err = -ENOMEM; + goto out_unregister_fwnode; + } + + raw_spin_lock_init(&its->lock); + mutex_init(&its->dev_alloc_lock); + INIT_LIST_HEAD(&its->entry); + INIT_LIST_HEAD(&its->its_device_list); + its->numa_node = NUMA_NO_NODE; + its->msi_domain_flags = IRQ_DOMAIN_FLAG_MSI_REMAP; + err = its_alloc_collections(its); + if (err) + goto out_free_its; + + err = its_init_domain(fwnode, its); + if (err) + goto out_free_collections; + + raw_spin_lock(&its_lock); + virtual_its_node = its; + list_add(&its->entry, &its_nodes); + raw_spin_unlock(&its_lock); + + return 0; + +out_free_collections: + its_free_collections(its); +out_free_its: + kvfree(its); +out_unregister_fwnode: + iort_deregister_domain_token(0); +out_free_fwnode: + irq_domain_free_fwnode(fwnode); + return err; + +} + static bool gic_rdists_supports_plpis(void) { return !!(gic_read_typer(gic_data_rdist_rd_base() + GICR_TYPER) & GICR_TYPER_PLPIS); @@ -5414,8 +5501,19 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists, its_acpi_probe(); if (list_empty(&its_nodes)) { - pr_warn("ITS: No ITS available, not enabling LPIs\n"); - return -ENXIO; + /* Initialize virtual ITS only if DirectLPI is set. */ + if (gic_rdists->has_direct_lpi) { + pr_info("ITS: No ITS available, using virtual ITS\n"); + err = virtual_its_init(); + if (err) { + pr_info("ITS: Virtual ITS creation fails\n"); + return err; + } + pr_info("ITS: Virtual ITS created\n"); + } else { + pr_warn("ITS: No ITS available, not enabling LPIs\n"); + return -ENXIO; + } } err = allocate_lpi_tables(); -- 2.30.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [RFC 2/2] irqchip/gic-v3-its: Introduce virtual ITS @ 2021-06-22 15:53 ` Boqun Feng 0 siblings, 0 replies; 12+ messages in thread From: Boqun Feng @ 2021-06-22 15:53 UTC (permalink / raw) To: Marc Zyngier Cc: Thomas Gleixner, linux-kernel, Arnd Bergmann, Bjorn Helgaas, Linux ARM, Catalin Marinas, Will Deacon, Boqun Feng, Sunil Muthuswamy GICv3 allows supporting LPI without an ITS, and in order to support such a platform, a virtual ITS is introduced. The virtual ITS has the same software part as a real ITS: having an irq domain, maintaining ->collections and maintaining the list of devices. The only difference is the virtual ITS doesn't have a backed ITS therefore it cannot issue ITS commands nor set up device tables. The virtual ITS only manages LPIs and the LPIs are configured via DirectLPI interfaces. And currently the virtual ITS is initialized only if there is no ITS in the system and yet DirectLPI is support. The virtual ITS approach provides the support for LPI without an ITS and reuses as much exisiting code as possible, and is the preparation for virtual PCI support on ARM64 Hyper-V guests. Co-developed-by: Sunil Muthuswamy <sunilmut@microsoft.com> Signed-off-by: Boqun Feng <boqun.feng@gmail.com> --- drivers/irqchip/irq-gic-v3-its.c | 114 ++++++++++++++++++++++++++++--- 1 file changed, 106 insertions(+), 8 deletions(-) diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c index 1916ac5d6371..4f2600377039 100644 --- a/drivers/irqchip/irq-gic-v3-its.c +++ b/drivers/irqchip/irq-gic-v3-its.c @@ -117,9 +117,16 @@ struct its_node { int vlpi_redist_offset; }; +/* + * LPI can be supported without ITS, in which case, a virtual its_node is + * initialized to allow configuring LPI with the DirectLPI approach. + */ +static struct its_node *virtual_its_node; + #define is_v4(its) (!!((its)->typer & GITS_TYPER_VLPIS)) #define is_v4_1(its) (!!((its)->typer & GITS_TYPER_VMAPP)) #define device_ids(its) (FIELD_GET(GITS_TYPER_DEVBITS, (its)->typer) + 1) +#define is_virtual(its) ((its) == virtual_its_node) #define ITS_ITT_ALIGN SZ_256 @@ -1096,6 +1103,10 @@ void name(struct its_node *its, \ unsigned long flags; \ u64 rd_idx; \ \ + /* Virtual ITS doesn't support ITS commands */ \ + if (is_virtual(its)) \ + return; \ + \ raw_spin_lock_irqsave(&its->lock, flags); \ \ cmd = its_allocate_entry(its); \ @@ -1464,7 +1475,8 @@ static void lpi_update_config(struct irq_data *d, u8 clr, u8 set) lpi_write_config(d, clr, set); if (gic_rdists->has_direct_lpi && - (is_v4_1(its_dev->its) || !irqd_is_forwarded_to_vcpu(d))) + (is_v4_1(its_dev->its) || !irqd_is_forwarded_to_vcpu(d) || + is_virtual(its_dev->its))) direct_lpi_inv(d); else if (!irqd_is_forwarded_to_vcpu(d)) its_send_inv(its_dev, its_get_event_id(d)); @@ -1690,6 +1702,10 @@ static void its_irq_compose_msi_msg(struct irq_data *d, struct msi_msg *msg) u64 addr; its = its_dev->its; + + /* Virtual ITS doesn't have ->get_msi_base function, skip */ + if (!its->get_msi_base) + return; addr = its->get_msi_base(its_dev); msg->address_lo = lower_32_bits(addr); @@ -3217,7 +3233,17 @@ static void its_cpu_init_collections(void) raw_spin_lock(&its_lock); list_for_each_entry(its, &its_nodes, entry) - its_cpu_init_collection(its); + /* + * Only initializes the software part of collections for virtual + * ITS. + */ + if (is_virtual(its)) { + int cpu = smp_processor_id(); + + its->collections[cpu].col_id = cpu; + } else { + its_cpu_init_collection(its); + } raw_spin_unlock(&its_lock); } @@ -3364,7 +3390,8 @@ static struct its_device *its_create_device(struct its_node *its, u32 dev_id, int nr_ites; int sz; - if (!its_alloc_device_table(its, dev_id)) + /* No need to allocate device table for virtual ITS */ + if (!is_virtual(its) && !its_alloc_device_table(its, dev_id)) return NULL; if (WARN_ON(!is_power_of_2(nvecs))) @@ -3551,9 +3578,12 @@ static int its_irq_domain_alloc(struct irq_domain *domain, unsigned int virq, if (err) return err; - err = iommu_dma_prepare_msi(info->desc, its->get_msi_base(its_dev)); - if (err) - return err; + /* Virtual ITS doesn't have ->get_msi_base function, skip */ + if (its->get_msi_base) { + err = iommu_dma_prepare_msi(info->desc, its->get_msi_base(its_dev)); + if (err) + return err; + } for (i = 0; i < nr_irqs; i++) { err = its_irq_gic_domain_alloc(domain, virq + i, hwirq + i); @@ -5121,6 +5151,63 @@ static int __init its_probe_one(struct resource *res, return err; } +static int __init virtual_its_init(void) +{ + struct its_node *its; + struct fwnode_handle *fwnode; + int err; + + fwnode = irq_domain_alloc_named_fwnode("Virtual ITS"); + if (!fwnode) + return -ENOMEM; + + /* + * Use 0 as the ID for virtual ITS, since we only init virtual ITS if + * there is no real ITS in the system, so it's fine. + */ + err = iort_register_domain_token(0, 0, fwnode); + if (err) + goto out_free_fwnode; + + its = kzalloc(sizeof(*its), GFP_KERNEL); + if (!its) { + err = -ENOMEM; + goto out_unregister_fwnode; + } + + raw_spin_lock_init(&its->lock); + mutex_init(&its->dev_alloc_lock); + INIT_LIST_HEAD(&its->entry); + INIT_LIST_HEAD(&its->its_device_list); + its->numa_node = NUMA_NO_NODE; + its->msi_domain_flags = IRQ_DOMAIN_FLAG_MSI_REMAP; + err = its_alloc_collections(its); + if (err) + goto out_free_its; + + err = its_init_domain(fwnode, its); + if (err) + goto out_free_collections; + + raw_spin_lock(&its_lock); + virtual_its_node = its; + list_add(&its->entry, &its_nodes); + raw_spin_unlock(&its_lock); + + return 0; + +out_free_collections: + its_free_collections(its); +out_free_its: + kvfree(its); +out_unregister_fwnode: + iort_deregister_domain_token(0); +out_free_fwnode: + irq_domain_free_fwnode(fwnode); + return err; + +} + static bool gic_rdists_supports_plpis(void) { return !!(gic_read_typer(gic_data_rdist_rd_base() + GICR_TYPER) & GICR_TYPER_PLPIS); @@ -5414,8 +5501,19 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists, its_acpi_probe(); if (list_empty(&its_nodes)) { - pr_warn("ITS: No ITS available, not enabling LPIs\n"); - return -ENXIO; + /* Initialize virtual ITS only if DirectLPI is set. */ + if (gic_rdists->has_direct_lpi) { + pr_info("ITS: No ITS available, using virtual ITS\n"); + err = virtual_its_init(); + if (err) { + pr_info("ITS: Virtual ITS creation fails\n"); + return err; + } + pr_info("ITS: Virtual ITS created\n"); + } else { + pr_warn("ITS: No ITS available, not enabling LPIs\n"); + return -ENXIO; + } } err = allocate_lpi_tables(); -- 2.30.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [RFC 2/2] irqchip/gic-v3-its: Introduce virtual ITS 2021-06-22 15:53 ` Boqun Feng @ 2021-06-22 17:17 ` Marc Zyngier -1 siblings, 0 replies; 12+ messages in thread From: Marc Zyngier @ 2021-06-22 17:17 UTC (permalink / raw) To: Boqun Feng Cc: Thomas Gleixner, linux-kernel, Arnd Bergmann, Bjorn Helgaas, Linux ARM, Catalin Marinas, Will Deacon, Sunil Muthuswamy On Tue, 22 Jun 2021 16:53:13 +0100, Boqun Feng <boqun.feng@gmail.com> wrote: > > GICv3 allows supporting LPI without an ITS, and in order to support > such a platform, a virtual ITS is introduced. The virtual ITS has the > same software part as a real ITS: having an irq domain, maintaining > ->collections and maintaining the list of devices. The only difference > is the virtual ITS doesn't have a backed ITS therefore it cannot issue > ITS commands nor set up device tables. The virtual ITS only manages LPIs > and the LPIs are configured via DirectLPI interfaces. That's not a virtual ITS. Not at all. It isn't even the shadow of an ITS. This is... something else. > > And currently the virtual ITS is initialized only if there is no ITS in > the system and yet DirectLPI is support. > > The virtual ITS approach provides the support for LPI without an ITS and > reuses as much exisiting code as possible, and is the preparation for > virtual PCI support on ARM64 Hyper-V guests. <rant> There is no translation, no isolation. This is a yet another sorry excuse for a hack. Why can't the Hyper-V folks implement the architecture, only the architecture, and all of it? </rant> > > Co-developed-by: Sunil Muthuswamy <sunilmut@microsoft.com> > Signed-off-by: Boqun Feng <boqun.feng@gmail.com> > --- > drivers/irqchip/irq-gic-v3-its.c | 114 ++++++++++++++++++++++++++++--- > 1 file changed, 106 insertions(+), 8 deletions(-) > > diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c > index 1916ac5d6371..4f2600377039 100644 > --- a/drivers/irqchip/irq-gic-v3-its.c > +++ b/drivers/irqchip/irq-gic-v3-its.c > @@ -117,9 +117,16 @@ struct its_node { > int vlpi_redist_offset; > }; > > +/* > + * LPI can be supported without ITS, in which case, a virtual its_node is > + * initialized to allow configuring LPI with the DirectLPI approach. > + */ > +static struct its_node *virtual_its_node; > + > #define is_v4(its) (!!((its)->typer & GITS_TYPER_VLPIS)) > #define is_v4_1(its) (!!((its)->typer & GITS_TYPER_VMAPP)) > #define device_ids(its) (FIELD_GET(GITS_TYPER_DEVBITS, (its)->typer) + 1) > +#define is_virtual(its) ((its) == virtual_its_node) And you can only have one? > > #define ITS_ITT_ALIGN SZ_256 > > @@ -1096,6 +1103,10 @@ void name(struct its_node *its, \ > unsigned long flags; \ > u64 rd_idx; \ > \ > + /* Virtual ITS doesn't support ITS commands */ \ > + if (is_virtual(its)) \ > + return; \ > + \ Oh gawd... > raw_spin_lock_irqsave(&its->lock, flags); \ > \ > cmd = its_allocate_entry(its); \ > @@ -1464,7 +1475,8 @@ static void lpi_update_config(struct irq_data *d, u8 clr, u8 set) > > lpi_write_config(d, clr, set); > if (gic_rdists->has_direct_lpi && > - (is_v4_1(its_dev->its) || !irqd_is_forwarded_to_vcpu(d))) > + (is_v4_1(its_dev->its) || !irqd_is_forwarded_to_vcpu(d) || > + is_virtual(its_dev->its))) > direct_lpi_inv(d); > else if (!irqd_is_forwarded_to_vcpu(d)) > its_send_inv(its_dev, its_get_event_id(d)); > @@ -1690,6 +1702,10 @@ static void its_irq_compose_msi_msg(struct irq_data *d, struct msi_msg *msg) > u64 addr; > > its = its_dev->its; > + > + /* Virtual ITS doesn't have ->get_msi_base function, skip */ > + if (!its->get_msi_base) > + return; So how do you target a redistributor? If you are going to use DirectLPI, this should hit the GICR_SETLPIR for the relevant redistributor. Actually, how do you target another redistributor? You can't send a MOVI, and you don't change the target address. And even if you could, how do you move the pending state from one pending table to another? Which means you probably have some other, non architectural stuff somewhere else. [...] I'll stop here. I'm not taking this hack built on top of the ITS code. Not now, not ever. If you really want to implement something that is outside of the scope of the architecture, do it outside of the GICv3 code, because this is just pretending to follow the architecture. Or even better, get the Hyper-V folks to implement a *real* virtual ITS, in the hypervisor. Yes, this is hard. M. -- Without deviation from the norm, progress is not possible. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC 2/2] irqchip/gic-v3-its: Introduce virtual ITS @ 2021-06-22 17:17 ` Marc Zyngier 0 siblings, 0 replies; 12+ messages in thread From: Marc Zyngier @ 2021-06-22 17:17 UTC (permalink / raw) To: Boqun Feng Cc: Thomas Gleixner, linux-kernel, Arnd Bergmann, Bjorn Helgaas, Linux ARM, Catalin Marinas, Will Deacon, Sunil Muthuswamy On Tue, 22 Jun 2021 16:53:13 +0100, Boqun Feng <boqun.feng@gmail.com> wrote: > > GICv3 allows supporting LPI without an ITS, and in order to support > such a platform, a virtual ITS is introduced. The virtual ITS has the > same software part as a real ITS: having an irq domain, maintaining > ->collections and maintaining the list of devices. The only difference > is the virtual ITS doesn't have a backed ITS therefore it cannot issue > ITS commands nor set up device tables. The virtual ITS only manages LPIs > and the LPIs are configured via DirectLPI interfaces. That's not a virtual ITS. Not at all. It isn't even the shadow of an ITS. This is... something else. > > And currently the virtual ITS is initialized only if there is no ITS in > the system and yet DirectLPI is support. > > The virtual ITS approach provides the support for LPI without an ITS and > reuses as much exisiting code as possible, and is the preparation for > virtual PCI support on ARM64 Hyper-V guests. <rant> There is no translation, no isolation. This is a yet another sorry excuse for a hack. Why can't the Hyper-V folks implement the architecture, only the architecture, and all of it? </rant> > > Co-developed-by: Sunil Muthuswamy <sunilmut@microsoft.com> > Signed-off-by: Boqun Feng <boqun.feng@gmail.com> > --- > drivers/irqchip/irq-gic-v3-its.c | 114 ++++++++++++++++++++++++++++--- > 1 file changed, 106 insertions(+), 8 deletions(-) > > diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c > index 1916ac5d6371..4f2600377039 100644 > --- a/drivers/irqchip/irq-gic-v3-its.c > +++ b/drivers/irqchip/irq-gic-v3-its.c > @@ -117,9 +117,16 @@ struct its_node { > int vlpi_redist_offset; > }; > > +/* > + * LPI can be supported without ITS, in which case, a virtual its_node is > + * initialized to allow configuring LPI with the DirectLPI approach. > + */ > +static struct its_node *virtual_its_node; > + > #define is_v4(its) (!!((its)->typer & GITS_TYPER_VLPIS)) > #define is_v4_1(its) (!!((its)->typer & GITS_TYPER_VMAPP)) > #define device_ids(its) (FIELD_GET(GITS_TYPER_DEVBITS, (its)->typer) + 1) > +#define is_virtual(its) ((its) == virtual_its_node) And you can only have one? > > #define ITS_ITT_ALIGN SZ_256 > > @@ -1096,6 +1103,10 @@ void name(struct its_node *its, \ > unsigned long flags; \ > u64 rd_idx; \ > \ > + /* Virtual ITS doesn't support ITS commands */ \ > + if (is_virtual(its)) \ > + return; \ > + \ Oh gawd... > raw_spin_lock_irqsave(&its->lock, flags); \ > \ > cmd = its_allocate_entry(its); \ > @@ -1464,7 +1475,8 @@ static void lpi_update_config(struct irq_data *d, u8 clr, u8 set) > > lpi_write_config(d, clr, set); > if (gic_rdists->has_direct_lpi && > - (is_v4_1(its_dev->its) || !irqd_is_forwarded_to_vcpu(d))) > + (is_v4_1(its_dev->its) || !irqd_is_forwarded_to_vcpu(d) || > + is_virtual(its_dev->its))) > direct_lpi_inv(d); > else if (!irqd_is_forwarded_to_vcpu(d)) > its_send_inv(its_dev, its_get_event_id(d)); > @@ -1690,6 +1702,10 @@ static void its_irq_compose_msi_msg(struct irq_data *d, struct msi_msg *msg) > u64 addr; > > its = its_dev->its; > + > + /* Virtual ITS doesn't have ->get_msi_base function, skip */ > + if (!its->get_msi_base) > + return; So how do you target a redistributor? If you are going to use DirectLPI, this should hit the GICR_SETLPIR for the relevant redistributor. Actually, how do you target another redistributor? You can't send a MOVI, and you don't change the target address. And even if you could, how do you move the pending state from one pending table to another? Which means you probably have some other, non architectural stuff somewhere else. [...] I'll stop here. I'm not taking this hack built on top of the ITS code. Not now, not ever. If you really want to implement something that is outside of the scope of the architecture, do it outside of the GICv3 code, because this is just pretending to follow the architecture. Or even better, get the Hyper-V folks to implement a *real* virtual ITS, in the hypervisor. Yes, this is hard. M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC 2/2] irqchip/gic-v3-its: Introduce virtual ITS 2021-06-22 15:53 ` Boqun Feng (?) (?) @ 2021-06-23 2:09 ` kernel test robot -1 siblings, 0 replies; 12+ messages in thread From: kernel test robot @ 2021-06-23 2:09 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 2027 bytes --] Hi Boqun, [FYI, it's a private test report for your RFC patch.] [auto build test ERROR on tip/irq/core] [also build test ERROR on v5.13-rc7 next-20210622] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Boqun-Feng/irqchip-gic-v3-its-Introduce-virtual-ITS/20210622-235606 base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 006ae1970a8cde1d3e92da69b324d12880133a13 config: arm64-randconfig-r023-20210622 (attached as .config) compiler: aarch64-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/9c3938edc614fd8eefac6b25fb9b02ddf90b7417 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Boqun-Feng/irqchip-gic-v3-its-Introduce-virtual-ITS/20210622-235606 git checkout 9c3938edc614fd8eefac6b25fb9b02ddf90b7417 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): aarch64-linux-ld: Unexpected GOT/PLT entries detected! aarch64-linux-ld: Unexpected run-time procedure linkages detected! aarch64-linux-ld: drivers/irqchip/irq-gic-v3-its.o: in function `its_init': irq-gic-v3-its.c:(.init.text+0x7e8): undefined reference to `iort_register_domain_token' >> aarch64-linux-ld: irq-gic-v3-its.c:(.init.text+0x940): undefined reference to `iort_deregister_domain_token' --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org [-- Attachment #2: config.gz --] [-- Type: application/gzip, Size: 39170 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC 2/2] irqchip/gic-v3-its: Introduce virtual ITS 2021-06-22 15:53 ` Boqun Feng ` (2 preceding siblings ...) (?) @ 2021-06-23 3:27 ` kernel test robot -1 siblings, 0 replies; 12+ messages in thread From: kernel test robot @ 2021-06-23 3:27 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 2086 bytes --] Hi Boqun, [FYI, it's a private test report for your RFC patch.] [auto build test ERROR on tip/irq/core] [also build test ERROR on v5.13-rc7 next-20210622] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Boqun-Feng/irqchip-gic-v3-its-Introduce-virtual-ITS/20210622-235606 base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 006ae1970a8cde1d3e92da69b324d12880133a13 config: arm-randconfig-p001-20210622 (attached as .config) compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/9c3938edc614fd8eefac6b25fb9b02ddf90b7417 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Boqun-Feng/irqchip-gic-v3-its-Introduce-virtual-ITS/20210622-235606 git checkout 9c3938edc614fd8eefac6b25fb9b02ddf90b7417 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): arm-linux-gnueabi-ld: warning: orphan section `__cpuidle_method_of_table' from `arch/arm/mach-omap2/pm33xx-core.o' being placed in section `__cpuidle_method_of_table' arm-linux-gnueabi-ld: drivers/irqchip/irq-gic-v3-its.o: in function `virtual_its_init': >> irq-gic-v3-its.c:(.init.text+0x15f8): undefined reference to `iort_register_domain_token' >> arm-linux-gnueabi-ld: irq-gic-v3-its.c:(.init.text+0x1bac): undefined reference to `iort_deregister_domain_token' --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org [-- Attachment #2: config.gz --] [-- Type: application/gzip, Size: 42041 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC 0/2] irqchip/gic-v3-its: Introduce virtual ITS 2021-06-22 15:53 ` Boqun Feng @ 2021-06-22 17:32 ` Marc Zyngier -1 siblings, 0 replies; 12+ messages in thread From: Marc Zyngier @ 2021-06-22 17:32 UTC (permalink / raw) To: Boqun Feng Cc: Thomas Gleixner, linux-kernel, Arnd Bergmann, Bjorn Helgaas, Linux ARM, Catalin Marinas, Will Deacon On Tue, 22 Jun 2021 16:53:11 +0100, Boqun Feng <boqun.feng@gmail.com> wrote: > > Hi Marc, > > Here is an RFC for supporting platforms having LPI supported but without > ITS. And this is for the virtual PCI support for ARM64 Hyper-V guests. > We currently choose this approach (LPI w/o ITS) because a) it's allowed > for GICv3 and b) ITS may not be a more efficient way to configure LPIs > compared to hypercalls, but we'd like to get feedbacks from the > community. > > Besides, patch #1 fixes a bug which I found while I was at it. > > Looking forwards to any comment and suggestion! My suggestion is to not do that. The ITS driver is used to drive an ITS. That, and only that. The simple fact that you mention hypercalls shows that this is *not* what the architecture allows. So if you are going to implement something that is evidently outside of the scope of the architecture, keep it in some Hyper-V specific code that doesn't involve the ITS driver. All you need is something that will piggy-back on top of the GIC driver using a hierarchical driver. We support that today. Of course, you'll have to invent your own firmware interface for discovery. Thanks, M. -- Without deviation from the norm, progress is not possible. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC 0/2] irqchip/gic-v3-its: Introduce virtual ITS @ 2021-06-22 17:32 ` Marc Zyngier 0 siblings, 0 replies; 12+ messages in thread From: Marc Zyngier @ 2021-06-22 17:32 UTC (permalink / raw) To: Boqun Feng Cc: Thomas Gleixner, linux-kernel, Arnd Bergmann, Bjorn Helgaas, Linux ARM, Catalin Marinas, Will Deacon On Tue, 22 Jun 2021 16:53:11 +0100, Boqun Feng <boqun.feng@gmail.com> wrote: > > Hi Marc, > > Here is an RFC for supporting platforms having LPI supported but without > ITS. And this is for the virtual PCI support for ARM64 Hyper-V guests. > We currently choose this approach (LPI w/o ITS) because a) it's allowed > for GICv3 and b) ITS may not be a more efficient way to configure LPIs > compared to hypercalls, but we'd like to get feedbacks from the > community. > > Besides, patch #1 fixes a bug which I found while I was at it. > > Looking forwards to any comment and suggestion! My suggestion is to not do that. The ITS driver is used to drive an ITS. That, and only that. The simple fact that you mention hypercalls shows that this is *not* what the architecture allows. So if you are going to implement something that is evidently outside of the scope of the architecture, keep it in some Hyper-V specific code that doesn't involve the ITS driver. All you need is something that will piggy-back on top of the GIC driver using a hierarchical driver. We support that today. Of course, you'll have to invent your own firmware interface for discovery. Thanks, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-06-23 3:27 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-22 15:53 [RFC 0/2] irqchip/gic-v3-its: Introduce virtual ITS Boqun Feng 2021-06-22 15:53 ` Boqun Feng 2021-06-22 15:53 ` [RFC 1/2] irqchip/gic-v3-its: Free collections if its domain initialization fails Boqun Feng 2021-06-22 15:53 ` Boqun Feng 2021-06-22 15:53 ` [RFC 2/2] irqchip/gic-v3-its: Introduce virtual ITS Boqun Feng 2021-06-22 15:53 ` Boqun Feng 2021-06-22 17:17 ` Marc Zyngier 2021-06-22 17:17 ` Marc Zyngier 2021-06-23 2:09 ` kernel test robot 2021-06-23 3:27 ` kernel test robot 2021-06-22 17:32 ` [RFC 0/2] " Marc Zyngier 2021-06-22 17:32 ` Marc Zyngier
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.