linux-kernel - Re: [PATCH RFC V2 02/10] irqchip: Add LoongArch CPU interrupt controller support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1c7b1ee5-3f1e-c090-fb2e-65741de76a9f@loongson.cn>
Date:   Thu, 2 Jun 2022 11:16:30 +0800
From:   Jianmin Lv <lvjianmin@...ngson.cn>
To:     Marc Zyngier <maz@...nel.org>
Cc:     Thomas Gleixner <tglx@...utronix.de>, linux-kernel@...r.kernel.org,
        Xuefeng Li <lixuefeng@...ngson.cn>,
        Huacai Chen <chenhuacai@...il.com>,
        Jiaxun Yang <jiaxun.yang@...goat.com>,
        Huacai Chen <chenhuacai@...ngson.cn>
Subject: Re: [PATCH RFC V2 02/10] irqchip: Add LoongArch CPU interrupt
 controller support

>>>> +
>>>> +int acpi_gsi_to_irq(u32 gsi, unsigned int *irqp)
>>>> +{
>>>> +	if (irqp != NULL)
>>>> +		*irqp = acpi_register_gsi(NULL, gsi, -1, -1);
>>>> +	return (*irqp >= 0) ? 0 : -EINVAL;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
>>>> +
>>>> +int acpi_isa_irq_to_gsi(unsigned int isa_irq, u32 *gsi)
>>>> +{
>>>> +	if (gsi)
>>>> +		*gsi = isa_irq;
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +/*
>>>> + * success: return IRQ number (>=0)
>>>> + * failure: return &lt; 0
>>>> + */
>>>> +int acpi_register_gsi(struct device *dev, u32 gsi, int trigger, int polarity)
>>>> +{
>>>> +	int id;
>>>> +	struct irq_fwspec fwspec;
>>>> +
>>>> +	switch (gsi) {
>>>> +	case GSI_MIN_CPU_IRQ ... GSI_MAX_CPU_IRQ:
>>>> +		fwspec.fwnode = liointc_domain->fwnode;
>>>> +		fwspec.param[0] = gsi - GSI_MIN_CPU_IRQ;
>>>> +		fwspec.param_count = 1;
>>>> +
>>>> +		return irq_create_fwspec_mapping(&amp;fwspec);
>>>> +
>>>> +	case GSI_MIN_LPC_IRQ ... GSI_MAX_LPC_IRQ:
>>>> +		if (!pch_lpc_domain)
>>>> +			return -EINVAL;
>>>> +
>>>> +		fwspec.fwnode = pch_lpc_domain->fwnode;
>>>> +		fwspec.param[0] = gsi - GSI_MIN_LPC_IRQ;
>>>> +		fwspec.param[1] = acpi_dev_get_irq_type(trigger, polarity);
>>>> +		fwspec.param_count = 2;
>>>> +
>>>> +		return irq_create_fwspec_mapping(&amp;fwspec);
>>>> +
>>>> +	case GSI_MIN_PCH_IRQ ... GSI_MAX_PCH_IRQ:
>>>> +		id = find_pch_pic(gsi);
>>>> +		if (id &lt; 0)
>>>> +			return -EINVAL;
>>>> +
>>>> +		fwspec.fwnode = pch_pic_domain[id]->fwnode;
>>>> +		fwspec.param[0] = gsi - acpi_pchpic[id]->gsi_base;
>>>> +		fwspec.param[1] = IRQ_TYPE_LEVEL_HIGH;
>>>> +		fwspec.param_count = 2;
>>>> +
>>>> +		return irq_create_fwspec_mapping(&amp;fwspec);
>>>> +	}
>>> So all the complexity here seems to stem from the fact that you deal
>>> with three ranges of interrupts, managed by three different pieces of
>>> code?
>>>
>> Yes.
>>
>>> Other architectures have similar requirements, and don't require to
>>> re-implement a private version of the ACPI API. Instead, they expose a
>>> single irqdomain, and deal with the various ranges internally.
>>>
>>> Clearly, not being able to reuse drivers/acpi/irq.c *is* an issue.
>>>
>> Thanks, I agree, that sounds a good and reasonable suggestion, and
>> I'll reserach it further and reuse code from drivers/acpi/irq.c as
>> can as possible.
>>
Hi, Marc, according to your suggestion, I carefully looked into gic 
driver of ARM, and I found one possible gsi mapping path as following:

acpi_register_gsi /* whatever the gsi is, gic domain for ARM is only 
single domain to use.*/
  ->irq_create_fwspec_mapping
    ->irq_find_mapping /* return irq in the mapping of irqdomain with 
fwnode_handle of acpi_gsi_domain_id if configured. */
    ->irq_domain_alloc_irqs /* if not configured and hierarchy domain */
      ->irq_domain_alloc_irqs_hierarchy
        ->domain->ops->alloc /* call gic_irq_domain_alloc */
          ->gic_irq_domain_map /* handle different GSI range as 
following code: */


         switch (__get_intid_range(hw)) {
         case SGI_RANGE:
         case PPI_RANGE:
         case EPPI_RANGE:
                 irq_set_percpu_devid(irq);
                 irq_domain_set_info(d, irq, hw, chip, d->host_data,
                                     handle_percpu_devid_irq, NULL, NULL);
                 break;

         case SPI_RANGE:
         case ESPI_RANGE:
                 irq_domain_set_info(d, irq, hw, chip, d->host_data,
                                     handle_fasteoi_irq, NULL, NULL);
                 irq_set_probe(irq);
                 irqd_set_single_target(irqd);
                 break;

         case LPI_RANGE:
                 if (!gic_dist_supports_lpis())
                         return -EPERM;
                 irq_domain_set_info(d, irq, hw, chip, d->host_data,
                                     handle_fasteoi_irq, NULL, NULL);
                 break;

Yes, it's well for ARM by this way, and I found that only one 
domain(specified by acpi_gsi_domain_id)is used.

But for LoongArch, different GSI range have to be handled in different 
domain(e.g. GSI for LIOINTC irqchip can be only mapped in LIOINTC 
domain.). The hwirq->irq mapping of different GSI range is stored in 
related separate domain. The reason leading to this is that an interrupt 
source is hardcodingly to connected to an interrupt vector for these 
irqchip(LIOINTC,LPC-PIC and PCH-PIC), and the interrupt source of them 
need to be configured with GSI in DSDT or FADT(e.g. SCI).

If only exposing one domain for LoongArch, when calling irq_find_mapping 
in acpi_register_gsi flow, the irq is returned only from the domain 
specfied by acpi_gsi_domain_id, so I'm afraid it's unsuitable to expose 
a single domain for acpi_register_gsi.

I'm so sorry, I really don't find a way to reuse driver/acpi/irq.c after 
my humble work.