Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1330112

Re: [PATCH 3/5] irqchip/gic-v2: Parse and export virtual GIC information

From Julien Grall <julien.grall@arm.com>
Newsgroups linux.kernel
Subject Re: [PATCH 3/5] irqchip/gic-v2: Parse and export virtual GIC information
Date 2016-02-09 12:30 +0100
Message-ID <r0eEy-3jd-17@gated-at.bofh.it> (permalink)
References <qZXaG-7Ql-21@gated-at.bofh.it> <qZXaG-7Ql-29@gated-at.bofh.it> <qZYT8-Dx-11@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On 08/02/16 18:30, Marc Zyngier wrote:
> Julien,

Hi Marc,

> On 08/02/16 16:47, Julien Grall wrote:

[...]

>> +static void __init gic_of_setup_kvm_info(struct device_node *node)
>> +{
>> +	int ret;
>> +	struct resource r;
>> +	unsigned int irq;
>> +
>> +	gic_v2_kvm_info.type = GIC_V2;
>> +
>> +	irq = irq_of_parse_and_map(node, 0);
>> +	if (!irq)
>> +		gic_v2_kvm_info.maint_irq = -1;
>
> Please don't do that. 0 *is* the value for an invalid interrupt, and
> this is what you should expose here. Same for GICv3.

I decided to use -1, because the function acpi_register_gsi is returning 
a negative value when an error occurred.

AFAICT, returning 0 would be a valid value for acpi_register_gsi.

>> +	else
>> +		gic_v2_kvm_info.maint_irq = irq;
>> +
>> +	ret = of_address_to_resource(node, 2, &r);
>> +	if (!ret) {
>> +		gic_v2_kvm_info.vctrl_base = r.start;
>> +		gic_v2_kvm_info.vctrl_size = resource_size(&r);
>> +	}
>> +
>> +	ret = of_address_to_resource(node, 3, &r);
>> +	if (!ret) {
>> +		if (!PAGE_ALIGNED(r.start))
>> +			pr_warn("GICV physical address 0x%llx not page aligned\n",
>> +				(unsigned long long)r.start);
>> +		else if (!PAGE_ALIGNED(resource_size(&r)))
>> +			pr_warn("GICV size 0x%llx not a multiple of page size 0x%lx\n",
>> +				(unsigned long long)resource_size(&r),
>> +				PAGE_SIZE);
>> +		else {
>> +			gic_v2_kvm_info.vcpu_base = r.start;
>> +			gic_v2_kvm_info.vcpu_size = resource_size(&r);
>
> This tends to make me think that this should actually be a proper
> resource, and not a set of arbitrary fields.

I will give a look.

[..]

>> @@ -1270,6 +1338,12 @@ gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
>>   		return -EINVAL;
>>
>>   	cpu_phy_base = gic_cpu_base;
>> +	acpi_data.maint_irq = processor->vgic_interrupt;
>> +	acpi_data.maint_irq_mode = (processor->flags & ACPI_MADT_VGIC_IRQ_MODE) ?
>> +				    ACPI_EDGE_SENSITIVE : ACPI_LEVEL_SENSITIVE;
>> +	acpi_data.vctrl_base = processor->gich_base_address;
>> +	acpi_data.vcpu_base = processor->gicv_base_address;
>> +
>
> Maybe you can now move all the ACPI data into this acpi_data structure?
> This would allow for slightly less clutter...

Good idea, I will do it in the next version.

[...]

>
> Thanks,

Regards,

-- 
Julien Grall

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 0/5] arm64: Add support of KVM with ACPI Julien Grall <julien.grall@arm.com> - 2016-02-08 17:50 +0100
  [PATCH 1/5] KVM: arm/arm64: arch_timer: Gather KVM specific information in a structure Julien Grall <julien.grall@arm.com> - 2016-02-08 17:50 +0100
  [PATCH 3/5] irqchip/gic-v2: Parse and export virtual GIC information Julien Grall <julien.grall@arm.com> - 2016-02-08 17:50 +0100
    Re: [PATCH 3/5] irqchip/gic-v2: Parse and export virtual GIC  information Marc Zyngier <marc.zyngier@arm.com> - 2016-02-08 19:40 +0100
      Re: [PATCH 3/5] irqchip/gic-v2: Parse and export virtual GIC  information Julien Grall <julien.grall@arm.com> - 2016-02-09 12:30 +0100
        Re: [PATCH 3/5] irqchip/gic-v2: Parse and export virtual GIC  information Marc Zyngier <marc.zyngier@arm.com> - 2016-02-09 12:40 +0100
    Re: [PATCH 3/5] irqchip/gic-v2: Parse and export virtual GIC  information Christoffer Dall <christoffer.dall@linaro.org> - 2016-02-09 21:50 +0100
      Re: [PATCH 3/5] irqchip/gic-v2: Parse and export virtual GIC  information Wei Huang <wei@redhat.com> - 2016-02-09 23:00 +0100
      Re: [PATCH 3/5] irqchip/gic-v2: Parse and export virtual GIC  information Julien Grall <julien.grall@arm.com> - 2016-02-10 15:20 +0100
        Re: [PATCH 3/5] irqchip/gic-v2: Parse and export virtual GIC  information Marc Zyngier <marc.zyngier@arm.com> - 2016-02-10 15:50 +0100
          Re: [PATCH 3/5] irqchip/gic-v2: Parse and export virtual GIC  information Julien Grall <julien.grall@arm.com> - 2016-02-10 16:30 +0100
  [PATCH 5/5] KVM: arm/arm64: vgic: Rely on the GIC driver to parse the firmware tables Julien Grall <julien.grall@arm.com> - 2016-02-08 17:50 +0100
  [PATCH 2/5] KVM: arm/arm64: arch_timer: Rely on the arch timer to parse the firmware tables Julien Grall <julien.grall@arm.com> - 2016-02-08 17:50 +0100
  [PATCH 4/5] irqchip/gic-v3: Parse and export virtual GIC information Julien Grall <julien.grall@arm.com> - 2016-02-08 17:50 +0100

csiph-web