From: Julien Grall <julien.grall@arm.com> To: Marc Zyngier <marc.zyngier@arm.com>, kvmarm@lists.cs.columbia.edu Cc: christoffer.dall@linaro.org, fu.wei@linaro.org, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, wei@redhat.com, al.stone@linaro.org, Thomas Gleixner <tglx@linutronix.de>, Jason Cooper <jason@lakedaemon.net> Subject: Re: [PATCH 3/5] irqchip/gic-v2: Parse and export virtual GIC information Date: Tue, 9 Feb 2016 11:23:55 +0000 [thread overview] Message-ID: <56B9CC4B.90403@arm.com> (raw) In-Reply-To: <56B8DEB8.4010509@arm.com> 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
WARNING: multiple messages have this Message-ID (diff)
From: julien.grall@arm.com (Julien Grall) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 3/5] irqchip/gic-v2: Parse and export virtual GIC information Date: Tue, 9 Feb 2016 11:23:55 +0000 [thread overview] Message-ID: <56B9CC4B.90403@arm.com> (raw) In-Reply-To: <56B8DEB8.4010509@arm.com> 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
next prev parent reply other threads:[~2016-02-09 11:24 UTC|newest] Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-02-08 16:47 [PATCH 0/5] arm64: Add support of KVM with ACPI Julien Grall 2016-02-08 16:47 ` Julien Grall 2016-02-08 16:47 ` [PATCH 1/5] KVM: arm/arm64: arch_timer: Gather KVM specific information in a structure Julien Grall 2016-02-08 16:47 ` Julien Grall 2016-02-08 16:47 ` Julien Grall 2016-02-08 16:47 ` [PATCH 2/5] KVM: arm/arm64: arch_timer: Rely on the arch timer to parse the firmware tables Julien Grall 2016-02-08 16:47 ` Julien Grall 2016-02-08 16:47 ` Julien Grall 2016-02-08 16:47 ` [PATCH 3/5] irqchip/gic-v2: Parse and export virtual GIC information Julien Grall 2016-02-08 16:47 ` Julien Grall 2016-02-08 16:47 ` Julien Grall 2016-02-08 18:30 ` Marc Zyngier 2016-02-08 18:30 ` Marc Zyngier 2016-02-08 18:30 ` Marc Zyngier 2016-02-09 11:23 ` Julien Grall [this message] 2016-02-09 11:23 ` Julien Grall 2016-02-09 11:31 ` Marc Zyngier 2016-02-09 11:31 ` Marc Zyngier 2016-02-09 11:31 ` Marc Zyngier 2016-02-09 20:49 ` Christoffer Dall 2016-02-09 20:49 ` Christoffer Dall 2016-02-09 20:49 ` Christoffer Dall 2016-02-09 21:57 ` Wei Huang 2016-02-09 21:57 ` Wei Huang 2016-02-09 21:57 ` Wei Huang 2016-02-10 14:19 ` Julien Grall 2016-02-10 14:19 ` Julien Grall 2016-02-10 14:46 ` Marc Zyngier 2016-02-10 14:46 ` Marc Zyngier 2016-02-10 14:46 ` Marc Zyngier 2016-02-10 15:22 ` Julien Grall 2016-02-10 15:22 ` Julien Grall 2016-02-08 16:47 ` [PATCH 4/5] irqchip/gic-v3: " Julien Grall 2016-02-08 16:47 ` Julien Grall 2016-02-08 16:47 ` Julien Grall 2016-02-08 16:47 ` [PATCH 5/5] KVM: arm/arm64: vgic: Rely on the GIC driver to parse the firmware tables Julien Grall 2016-02-08 16:47 ` Julien Grall 2016-02-08 16:47 ` Julien Grall
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=56B9CC4B.90403@arm.com \ --to=julien.grall@arm.com \ --cc=al.stone@linaro.org \ --cc=christoffer.dall@linaro.org \ --cc=fu.wei@linaro.org \ --cc=jason@lakedaemon.net \ --cc=kvm@vger.kernel.org \ --cc=kvmarm@lists.cs.columbia.edu \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=marc.zyngier@arm.com \ --cc=tglx@linutronix.de \ --cc=wei@redhat.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.