Re: [PATCH 2/4] kvmtool: ARM: allow level interrupts in device tree
Hi,
On 11/12/14 17:15, Will Deacon wrote:
> On Thu, Dec 11, 2014 at 04:30:33PM +, Andre Przywara wrote:
>> Currently we describe every interrupt for each device in the FDT
>> as being edge triggered.
>> Add a parameter to the irq property generation to allow devices to
>> specify their interrupts as level triggered if needed.
>>
>> Signed-off-by: Andre Przywara
>> ---
>> tools/kvm/arm/fdt.c|6 +++---
>> tools/kvm/hw/serial.c |5 +++--
>> tools/kvm/include/kvm/ioport.h |4 +++-
>> tools/kvm/ioport.c |6 --
>> tools/kvm/virtio/mmio.c|5 +++--
>> 5 files changed, 16 insertions(+), 10 deletions(-)
>
> [...]
>
>> @@ -71,7 +72,8 @@ static void generate_ioport_fdt_node(void *fdt,
>> static void generate_ioport_fdt_node(void *fdt,
>> struct device_header *dev_hdr,
>> void (*generate_irq_prop)(void *fdt,
>> - u8 irq))
>> + u8 irq,
>> + u32 irq_type))
>> {
>> die("Unable to generate device tree nodes without libfdt\n");
>> }
>> diff --git a/tools/kvm/virtio/mmio.c b/tools/kvm/virtio/mmio.c
>> index 3a2bd62..28b0651 100644
>> --- a/tools/kvm/virtio/mmio.c
>> +++ b/tools/kvm/virtio/mmio.c
>> @@ -233,7 +233,8 @@ static void virtio_mmio_mmio_callback(struct kvm_cpu
>> *vcpu,
>> static void generate_virtio_mmio_fdt_node(void *fdt,
>>struct device_header *dev_hdr,
>>void (*generate_irq_prop)(void *fdt,
>> -u8 irq))
>> +u8 irq,
>> +u32 type))
>> {
>> char dev_name[DEVICE_NAME_MAX_LEN];
>> struct virtio_mmio *vmmio = container_of(dev_hdr,
>> @@ -250,7 +251,7 @@ static void generate_virtio_mmio_fdt_node(void *fdt,
>> _FDT(fdt_begin_node(fdt, dev_name));
>> _FDT(fdt_property_string(fdt, "compatible", "virtio,mmio"));
>> _FDT(fdt_property(fdt, "reg", reg_prop, sizeof(reg_prop)));
>> -generate_irq_prop(fdt, vmmio->irq);
>> +generate_irq_prop(fdt, vmmio->irq, GIC_FDT_IRQ_FLAGS_EDGE_LO_HI);
>
> This is a GIC-specific #define in arch-agnostic code. I think we should have
> a new enum type for describing edge and level interrupts, then just use
> that instead.
Right, good point.
I dug a bit in the kernel source, and actually those values behind the
defines are not GIC specific, but are generic IRQ FDT values (used by
other architectures as well). Even the GIC driver source uses these
defines, it's just the GICv3 binding doc that speaks of those specific
numbers.
So if you don't mind, I will remove the GIC_FDT_IRQ_FLAGS_{LEVEL,EDGE}
defines from kvmtool's headers and use the generic defines from the
kernel in one of the fdt headers.
Cheers,
Andre.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] kvmtool: ARM: allow level interrupts in device tree
On Thu, Dec 11, 2014 at 04:30:33PM +, Andre Przywara wrote:
> Currently we describe every interrupt for each device in the FDT
> as being edge triggered.
> Add a parameter to the irq property generation to allow devices to
> specify their interrupts as level triggered if needed.
>
> Signed-off-by: Andre Przywara
> ---
> tools/kvm/arm/fdt.c|6 +++---
> tools/kvm/hw/serial.c |5 +++--
> tools/kvm/include/kvm/ioport.h |4 +++-
> tools/kvm/ioport.c |6 --
> tools/kvm/virtio/mmio.c|5 +++--
> 5 files changed, 16 insertions(+), 10 deletions(-)
[...]
> @@ -71,7 +72,8 @@ static void generate_ioport_fdt_node(void *fdt,
> static void generate_ioport_fdt_node(void *fdt,
>struct device_header *dev_hdr,
>void (*generate_irq_prop)(void *fdt,
> -u8 irq))
> +u8 irq,
> +u32 irq_type))
> {
> die("Unable to generate device tree nodes without libfdt\n");
> }
> diff --git a/tools/kvm/virtio/mmio.c b/tools/kvm/virtio/mmio.c
> index 3a2bd62..28b0651 100644
> --- a/tools/kvm/virtio/mmio.c
> +++ b/tools/kvm/virtio/mmio.c
> @@ -233,7 +233,8 @@ static void virtio_mmio_mmio_callback(struct kvm_cpu
> *vcpu,
> static void generate_virtio_mmio_fdt_node(void *fdt,
> struct device_header *dev_hdr,
> void (*generate_irq_prop)(void *fdt,
> - u8 irq))
> + u8 irq,
> + u32 type))
> {
> char dev_name[DEVICE_NAME_MAX_LEN];
> struct virtio_mmio *vmmio = container_of(dev_hdr,
> @@ -250,7 +251,7 @@ static void generate_virtio_mmio_fdt_node(void *fdt,
> _FDT(fdt_begin_node(fdt, dev_name));
> _FDT(fdt_property_string(fdt, "compatible", "virtio,mmio"));
> _FDT(fdt_property(fdt, "reg", reg_prop, sizeof(reg_prop)));
> - generate_irq_prop(fdt, vmmio->irq);
> + generate_irq_prop(fdt, vmmio->irq, GIC_FDT_IRQ_FLAGS_EDGE_LO_HI);
This is a GIC-specific #define in arch-agnostic code. I think we should have
a new enum type for describing edge and level interrupts, then just use
that instead.
Will
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] kvmtool: ARM: allow level interrupts in device tree
Currently we describe every interrupt for each device in the FDT
as being edge triggered.
Add a parameter to the irq property generation to allow devices to
specify their interrupts as level triggered if needed.
Signed-off-by: Andre Przywara
---
tools/kvm/arm/fdt.c|6 +++---
tools/kvm/hw/serial.c |5 +++--
tools/kvm/include/kvm/ioport.h |4 +++-
tools/kvm/ioport.c |6 --
tools/kvm/virtio/mmio.c|5 +++--
5 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/tools/kvm/arm/fdt.c b/tools/kvm/arm/fdt.c
index 4a33846..918d89f 100644
--- a/tools/kvm/arm/fdt.c
+++ b/tools/kvm/arm/fdt.c
@@ -74,12 +74,12 @@ static void generate_cpu_nodes(void *fdt, struct kvm *kvm)
_FDT(fdt_end_node(fdt));
}
-static void generate_irq_prop(void *fdt, u8 irq)
+static void generate_irq_prop(void *fdt, u8 irq, u32 irq_type)
{
u32 irq_prop[] = {
cpu_to_fdt32(GIC_FDT_IRQ_TYPE_SPI),
cpu_to_fdt32(irq - GIC_SPI_IRQ_BASE),
- cpu_to_fdt32(GIC_FDT_IRQ_FLAGS_EDGE_LO_HI),
+ cpu_to_fdt32(irq_type)
};
_FDT(fdt_property(fdt, "interrupts", irq_prop, sizeof(irq_prop)));
@@ -127,7 +127,7 @@ static int setup_fdt(struct kvm *kvm)
void *fdt_dest = guest_flat_to_host(kvm,
kvm->arch.dtb_guest_start);
void (*generate_mmio_fdt_nodes)(void *, struct device_header *,
- void (*)(void *, u8));
+ void (*)(void *, u8, u32));
void (*generate_cpu_peripheral_fdt_nodes)(void *, struct kvm *, u32)
= kvm->cpus[0]->generate_fdt_nodes;
diff --git a/tools/kvm/hw/serial.c b/tools/kvm/hw/serial.c
index 60147de..266863b 100644
--- a/tools/kvm/hw/serial.c
+++ b/tools/kvm/hw/serial.c
@@ -369,7 +369,8 @@ static bool serial8250_in(struct ioport *ioport, struct
kvm_cpu *vcpu, u16 port,
#define DEVICE_NAME_MAX_LEN 32
static void serial8250_generate_fdt_node(struct ioport *ioport, void *fdt,
void (*generate_irq_prop)(void *fdt,
- u8 irq))
+ u8 irq,
+ u32 type))
{
char dev_name[DEVICE_NAME_MAX_LEN];
struct serial8250_device *dev = ioport->priv;
@@ -384,7 +385,7 @@ static void serial8250_generate_fdt_node(struct ioport
*ioport, void *fdt,
_FDT(fdt_begin_node(fdt, dev_name));
_FDT(fdt_property_string(fdt, "compatible", "ns16550a"));
_FDT(fdt_property(fdt, "reg", reg_prop, sizeof(reg_prop)));
- generate_irq_prop(fdt, dev->irq);
+ generate_irq_prop(fdt, dev->irq, GIC_FDT_IRQ_FLAGS_EDGE_LO_HI);
_FDT(fdt_property_cell(fdt, "clock-frequency", 1843200));
_FDT(fdt_end_node(fdt));
}
diff --git a/tools/kvm/include/kvm/ioport.h b/tools/kvm/include/kvm/ioport.h
index 18da47c..ffd938d 100644
--- a/tools/kvm/include/kvm/ioport.h
+++ b/tools/kvm/include/kvm/ioport.h
@@ -31,7 +31,9 @@ struct ioport_operations {
bool (*io_in)(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port,
void *data, int size);
bool (*io_out)(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port,
void *data, int size);
void (*generate_fdt_node)(struct ioport *ioport, void *fdt,
- void (*generate_irq_prop)(void *fdt, u8 irq));
+ void (*generate_irq_prop)(void *fdt,
+ u8 irq,
+ u32 irq_type));
};
void ioport__setup_arch(struct kvm *kvm);
diff --git a/tools/kvm/ioport.c b/tools/kvm/ioport.c
index 5bfb7e2..b507cab 100644
--- a/tools/kvm/ioport.c
+++ b/tools/kvm/ioport.c
@@ -59,7 +59,8 @@ static void ioport_remove(struct rb_root *root, struct ioport
*data)
static void generate_ioport_fdt_node(void *fdt,
struct device_header *dev_hdr,
void (*generate_irq_prop)(void *fdt,
- u8 irq))
+ u8 irq,
+ u32 irq_type))
{
struct ioport *ioport = container_of(dev_hdr, struct ioport, dev_hdr);
struct ioport_operations *ops = ioport->ops;
@@ -71,7 +72,8 @@ static void generate_ioport_fdt_node(void *fdt,
static void generate_ioport_fdt_node(void *fdt,
struct device_header *dev_hdr,
void (*generate_irq_prop)(void *fdt,
- u8 irq))
+
