Tom Lyon | 1 Oct 2010 01:09
Picon
Favicon

Re: [PATCH 3/3] VFIO V4: VFIO driver: Non-privileged user level PCI drivers

On Monday, September 27, 2010 04:54:21 am Michael S. Tsirkin wrote:
> On Wed, Sep 22, 2010 at 02:18:24PM -0700, Tom Lyon wrote:
> > Signed-off-by: Tom Lyon <pugs <at> cisco.com>
> 
> Some comments on the pci bits:
> 
> After going over them for the Nth time - something needs to be done
> with the rvirt/write tables. I doubt anyone besides me and you
> has gone over them: one has to pull
> up the spec just to understand which bits are set here. Why can't we
> have a module init routine and use the macros in pci_regs.h for this
> purpose, as all other drivers do? We will also get to use the nice
> endian-ness macros linux has for portability ...

I tried a couple of approaches before settling on this one. Yes, its dense, 
perhaps its ugly, but a table approach beats open coding of rules.
And I absolutely don't expect it to verifiable to someone without a PCI spec;
I had to have the PCI spec to write it! This is an example of where 
correctness and readability pull to different places. And the defines in 
pci_regs are incomplete and not real self-consistent anyways.

Perhaps its time for someone to code up a different approach? Code talks!

> 
> > +static struct perm_bits pci_cap_basic_perm[] = {
> 
> What endian-ness is this? Native?
> You pass this to user as is but pci config is little endian...
> Also -are all these readonly? So const? read mostly?
Yes, I need some consts.
(Continue reading)

Thomas Gleixner | 1 Oct 2010 01:14
Picon

[patch 03/47] genirq: Provide status modifier

Provide a irq_desc.status modifier function to cleanup the direct
access to irq_desc in arch and driver code.

Signed-off-by: Thomas Gleixner <tglx <at> linutronix.de>
---
 include/linux/irq.h |   27 +++++++++++++++++++++++++--
 kernel/irq/chip.c   |   26 +++++++-------------------
 2 files changed, 32 insertions(+), 21 deletions(-)

Index: linux-2.6-tip/include/linux/irq.h
===================================================================
--- linux-2.6-tip.orig/include/linux/irq.h
+++ linux-2.6-tip/include/linux/irq.h
 <at>  <at>  -72,6 +72,10  <at>  <at>  typedef	void (*irq_flow_handler_t)(unsig
 #define IRQ_ONESHOT		0x08000000	/* IRQ is not unmasked after hardirq */
 #define IRQ_NESTED_THREAD	0x10000000	/* IRQ is nested into another, no own handler thread */

+#define IRQF_MODIFY_MASK	\
+	(IRQ_TYPE_SENSE_MASK | IRQ_NOPROBE | IRQ_NOREQUEST | \
+	 IRQ_NOAUTOEN | IRQ_MOVE_PCNTXT | IRQ_LEVEL)
+
 #ifdef CONFIG_IRQ_PER_CPU
 # define CHECK_IRQ_PER_CPU(var) ((var) & IRQ_PER_CPU)
 # define IRQ_NO_BALANCING_MASK	(IRQ_PER_CPU | IRQ_NO_BALANCING)
 <at>  <at>  -428,8 +432,27  <at>  <at>  set_irq_chained_handler(unsigned int irq

 extern void set_irq_nested_thread(unsigned int irq, int nest);

-extern void set_irq_noprobe(unsigned int irq);
-extern void set_irq_probe(unsigned int irq);
(Continue reading)

Thomas Gleixner | 1 Oct 2010 01:14
Picon

[patch 04/47] arm: Use irq status modifier

Replace the open coded unlocked access to irq_desc.status

Signed-off-by: Thomas Gleixner <tglx <at> linutronix.de>
---
 arch/arm/kernel/irq.c |   20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

Index: linux-2.6-tip/arch/arm/kernel/irq.c
===================================================================
--- linux-2.6-tip.orig/arch/arm/kernel/irq.c
+++ linux-2.6-tip/arch/arm/kernel/irq.c
 <at>  <at>  -132,24 +132,18  <at>  <at>  asmlinkage void __exception asm_do_IRQ(u

 void set_irq_flags(unsigned int irq, unsigned int iflags)
 {
-	struct irq_desc *desc;
-	unsigned long flags;
+	unsigned long clr = 0;

-	if (irq >= nr_irqs) {
-		printk(KERN_ERR "Trying to set irq flags for IRQ%d\n", irq);
-		return;
-	}
+	irq_set_status_flags(irq, IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN);

-	desc = irq_to_desc(irq);
-	raw_spin_lock_irqsave(&desc->lock, flags);
-	desc->status |= IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN;
 	if (iflags & IRQF_VALID)
-		desc->status &= ~IRQ_NOREQUEST;
(Continue reading)

Thomas Gleixner | 1 Oct 2010 01:14
Picon

[patch 05/47] genirq-sanitize-irq-data-accessors.patch

Signed-off-by: Thomas Gleixner <tglx <at> linutronix.de>
---
 include/linux/irq.h |   31 ++++++++++++++++++++++++++-----
 kernel/irq/chip.c   |    8 ++++++++
 2 files changed, 34 insertions(+), 5 deletions(-)

Index: linux-2.6-tip/include/linux/irq.h
===================================================================
--- linux-2.6-tip.orig/include/linux/irq.h
+++ linux-2.6-tip/include/linux/irq.h
 <at>  <at>  -478,17 +478,38  <at>  <at>  extern int set_irq_data(unsigned int irq
 extern int set_irq_chip_data(unsigned int irq, void *data);
 extern int set_irq_type(unsigned int irq, unsigned int type);
 extern int set_irq_msi(unsigned int irq, struct msi_desc *entry);
+extern struct irq_data *irq_get_irq_data(unsigned int irq);

+static inline struct irq_chip *get_irq_chip(unsigned int irq)
+{
+	struct irq_data *d = irq_get_irq_data(irq);
+	return d ? d->chip : NULL;
+}
+
+static inline void *get_irq_chip_data(unsigned int irq)
+{
+	struct irq_data *d = irq_get_irq_data(irq);
+	return d ? d->chip_data : NULL;
+}
+
+static inline void *get_irq_data(unsigned int irq)
+{
(Continue reading)

Thomas Gleixner | 1 Oct 2010 01:15
Picon

[patch 06/47] genirq: Distangle kernel/irq/handle.c

kernel/irq/handle.c has become a dumpground for random code in random
order. Split out the irq descriptor management and the dummy irq_chip
implementation into separate files. Cleanup the include maze while at
it.

No code change.

Signed-off-by: Thomas Gleixner <tglx <at> linutronix.de>
---
 kernel/irq/Makefile    |    2 
 kernel/irq/dummychip.c |   67 +++++++++
 kernel/irq/handle.c    |  344 -------------------------------------------------
 kernel/irq/irqdesc.c   |  281 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 351 insertions(+), 343 deletions(-)

Index: linux-2.6-tip/kernel/irq/Makefile
===================================================================
--- linux-2.6-tip.orig/kernel/irq/Makefile
+++ linux-2.6-tip/kernel/irq/Makefile
 <at>  <at>  -1,5 +1,5  <at>  <at> 

-obj-y := handle.o manage.o spurious.o resend.o chip.o devres.o
+obj-y := irqdesc.o handle.o manage.o spurious.o resend.o chip.o dummychip.o devres.o
 obj-$(CONFIG_GENERIC_IRQ_PROBE) += autoprobe.o
 obj-$(CONFIG_PROC_FS) += proc.o
 obj-$(CONFIG_GENERIC_PENDING_IRQ) += migration.o
Index: linux-2.6-tip/kernel/irq/dummychip.c
===================================================================
--- /dev/null
+++ linux-2.6-tip/kernel/irq/dummychip.c
(Continue reading)

Thomas Gleixner | 1 Oct 2010 01:15
Picon

[patch 11/47] genirq: Provide default irq init flags

Arch code sets it's own irq_desc.status flags right after boot and for
dynamically allocated interrupts. That might involve iterating over a
huge array.

Allow ARCH_IRQ_INIT_FLAGS to set separate flags aside of IRQ_DISABLED
which is the default.

Signed-off-by: Thomas Gleixner <tglx <at> linutronix.de>
---
 include/linux/irq.h  |    6 ++++++
 kernel/irq/chip.c    |    2 +-
 kernel/irq/irqdesc.c |    6 +++---
 3 files changed, 10 insertions(+), 4 deletions(-)

Index: linux-2.6-tip/include/linux/irq.h
===================================================================
--- linux-2.6-tip.orig/include/linux/irq.h
+++ linux-2.6-tip/include/linux/irq.h
 <at>  <at>  -281,6 +281,12  <at>  <at>  extern struct irq_desc *irq_to_desc_allo
  */
 #include <asm/hw_irq.h>

+#ifndef ARCH_IRQ_INIT_FLAGS
+# define ARCH_IRQ_INIT_FLAGS	0
+#endif
+
+#define IRQ_DEFAULT_INIT_FLAGS	(IRQ_DISABLED | ARCH_IRQ_INIT_FLAGS)
+
 extern int setup_irq(unsigned int irq, struct irqaction *new);
 extern void remove_irq(unsigned int irq, struct irqaction *act);
(Continue reading)

Thomas Gleixner | 1 Oct 2010 01:15
Picon

[patch 14/47] genirq: Implement a sane sparse_irq allocator

The current sparse_irq allocator has several short comings due to
failures in the design or the lack of it:

 - Requires iteration over the number of active irqs to find a free slot
   (Some architectures have grown their own workarounds for this)
 - Removal of entries is not possible
 - Racy between create_irq_nr and destroy_irq (plugged by horrible
   callbacks)
 - Migration of active irq descriptors is not possible
 - No bulk allocation of irq ranges
 - Sprinkeled irq_desc references all over the place outside of kernel/irq/
   (The previous chip functions series is addressing this issue)

Implement a sane allocator which fixes the above short comings (though
migration of active descriptors needs a full tree wide cleanup of the
direct and mostly unlocked access to irq_desc).

The new allocator still uses a radix_tree, but uses a bitmap for
keeping track of allocated irq numbers. That allows:

 - Fast lookup of a free slot
 - Allows the removal of descriptors
 - Prevents the create/destroy race
 - Bulk allocation of consecutive irq ranges
 - Basic design is ready for migration of life descriptors after
   further cleanups

The bitmap is also used in the SPARSE_IRQ=n case for lookup and
raceless (de)allocation of irq numbers. So it removes the requirement
for looping through the descriptor array to find slots.
(Continue reading)

Thomas Gleixner | 1 Oct 2010 01:15
Picon

[patch 13/47] powerpc: Use ARCH_IRQ_INIT_FLAGS

Define the ARCH_IRQ_INIT_FLAGS instead of fixing it up in a loop.

Signed-off-by: Thomas Gleixner <tglx <at> linutronix.de>
---
 arch/powerpc/include/asm/hw_irq.h |    2 ++
 arch/powerpc/kernel/irq.c         |   15 ---------------
 2 files changed, 2 insertions(+), 15 deletions(-)

Index: linux-2.6-tip/arch/powerpc/include/asm/hw_irq.h
===================================================================
--- linux-2.6-tip.orig/arch/powerpc/include/asm/hw_irq.h
+++ linux-2.6-tip/arch/powerpc/include/asm/hw_irq.h
 <at>  <at>  -124,6 +124,8  <at>  <at>  static inline int irqs_disabled_flags(un

 #endif /* CONFIG_PPC64 */

+#define ARCH_IRQ_INIT_FLAGS	IRQ_NOREQUEST
+
 /*
  * interrupt-retrigger: should we handle this via lost interrupts and IPIs
  * or should we not care like we do now ? --BenH.
Index: linux-2.6-tip/arch/powerpc/kernel/irq.c
===================================================================
--- linux-2.6-tip.orig/arch/powerpc/kernel/irq.c
+++ linux-2.6-tip/arch/powerpc/kernel/irq.c
 <at>  <at>  -1072,21 +1072,6  <at>  <at>  void irq_free_virt(unsigned int virq, un

 int arch_early_irq_init(void)
 {
-	struct irq_desc *desc;
(Continue reading)

Thomas Gleixner | 1 Oct 2010 01:16
Picon

[patch 24/47] x86: i8259: Convert to new irq_chip functions

Signed-off-by: Thomas Gleixner <tglx <at> linutronix.de>
---
 arch/x86/include/asm/i8259.h   |    2 +
 arch/x86/kernel/apic/io_apic.c |   20 ++++++-------
 arch/x86/kernel/apic/nmi.c     |    2 -
 arch/x86/kernel/i8259.c        |   63 ++++++++++++++++++++---------------------
 arch/x86/kernel/smpboot.c      |    4 +-
 5 files changed, 47 insertions(+), 44 deletions(-)

Index: linux-2.6-tip/arch/x86/include/asm/i8259.h
===================================================================
--- linux-2.6-tip.orig/arch/x86/include/asm/i8259.h
+++ linux-2.6-tip/arch/x86/include/asm/i8259.h
 <at>  <at>  -55,6 +55,8  <at>  <at>  extern struct irq_chip i8259A_chip;
 struct legacy_pic {
 	int nr_legacy_irqs;
 	struct irq_chip *chip;
+	void (*mask)(unsigned int irq);
+	void (*unmask)(unsigned int irq);
 	void (*mask_all)(void);
 	void (*restore_mask)(void);
 	void (*init)(int auto_eoi);
Index: linux-2.6-tip/arch/x86/kernel/apic/io_apic.c
===================================================================
--- linux-2.6-tip.orig/arch/x86/kernel/apic/io_apic.c
+++ linux-2.6-tip/arch/x86/kernel/apic/io_apic.c
 <at>  <at>  -1474,7 +1474,7  <at>  <at>  static void setup_IO_APIC_irq(int apic_i

 	ioapic_register_intr(irq, desc, trigger);
 	if (irq < legacy_pic->nr_legacy_irqs)
(Continue reading)

Thomas Gleixner | 1 Oct 2010 01:16
Picon

[patch 26/47] x86: io_apic: Convert startup to new irq_chip function

Signed-off-by: Thomas Gleixner <tglx <at> linutronix.de>
---
 arch/x86/kernel/apic/io_apic.c |   12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

Index: linux-2.6-tip/arch/x86/kernel/apic/io_apic.c
===================================================================
--- linux-2.6-tip.orig/arch/x86/kernel/apic/io_apic.c
+++ linux-2.6-tip/arch/x86/kernel/apic/io_apic.c
 <at>  <at>  -2232,11 +2232,10  <at>  <at>  static int __init timer_irq_works(void)
  * an edge even if it isn't on the 8259A...
  */

-static unsigned int startup_ioapic_irq(unsigned int irq)
+static unsigned int startup_ioapic_irq(struct irq_data *data)
 {
-	int was_pending = 0;
+	int was_pending = 0, irq = data->irq;
 	unsigned long flags;
-	struct irq_cfg *cfg;

 	raw_spin_lock_irqsave(&ioapic_lock, flags);
 	if (irq < legacy_pic->nr_legacy_irqs) {
 <at>  <at>  -2244,8 +2243,7  <at>  <at>  static unsigned int startup_ioapic_irq(u
 		if (legacy_pic->irq_pending(irq))
 			was_pending = 1;
 	}
-	cfg = irq_cfg(irq);
-	__unmask_ioapic(cfg);
+	__unmask_ioapic(data->chip_data);
(Continue reading)


Gmane