>>> On 09.05.11 at 12:43, Ian Campbell <ian.campbell@xxxxxxxxxx> wrote:
> # HG changeset patch
> # User Ian Campbell <ian.campbell@xxxxxxxxxx>
> # Date 1304937815 -3600
> # Node ID 35abcbcdf8bcabab6e0bbd929f69b613e167edfd
> # Parent 4b0692880dfa557d4e1537c7a58c412c1286a416
> xen: ioapic: avoid gcc 4.6 warnings about uninitialised variables
Seems like this got lost, but it really doesn't only fix a compiler
warning, because (not mentioned in the description) ...
> gcc 4.6 complains:
> io_apic.c: In function 'restore_IO_APIC_setup':
>
> /build/user-xen_4.1.0-3-amd64-zSon7K/xen-4.1.0/debian/build/build-hyperviso
> r_amd64_amd64/xen/include/asm/io_apic.h:150:26: error: '*((void *)&entry+4)'
> may be used uninitialized in this function [-Werror=uninitialized]
> io_apic.c:221:32: note: '*((void *)&entry+4)' was declared here
>
> /build/user-xen_4.1.0-3-amd64-zSon7K/xen-4.1.0/debian/build/build-hyperviso
> r_amd64_amd64/xen/include/asm/io_apic.h:150:26: error: 'entry' may be used
> uninitialized in this function [-Werror=uninitialized]
> io_apic.c:221:32: note: 'entry' was declared here
> cc1: all warnings being treated as errors
>
> Add functions to read/write an entire IO APIC entry using an explicit
> union to allow gcc to spot the initialisation.
>
> Reported as Debian bug #625438, thanks to Matthias Klose.
>
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
Acked-by: Jan Beulich <jbeulich@xxxxxxxxxx>
> diff -r 4b0692880dfa -r 35abcbcdf8bc xen/arch/x86/io_apic.c
> --- a/xen/arch/x86/io_apic.c Thu May 05 17:40:34 2011 +0100
> +++ b/xen/arch/x86/io_apic.c Mon May 09 11:43:35 2011 +0100
> @@ -156,6 +156,52 @@ nomem:
> return 0;
> }
>
> +union entry_union {
> + struct { u32 w1, w2; };
> + struct IO_APIC_route_entry entry;
> +};
> +
> +static struct IO_APIC_route_entry __ioapic_read_entry(int apic, int pin,
> int raw)
> +{
> + unsigned int (*read)(unsigned int, unsigned int)
> + = raw ? __io_apic_read : io_apic_read;
> + union entry_union eu;
> + eu.w1 = (*read)(apic, 0x10 + 2 * pin);
> + eu.w2 = (*read)(apic, 0x11 + 2 * pin);
> + return eu.entry;
> +}
> +
> +static struct IO_APIC_route_entry ioapic_read_entry(int apic, int pin, int
> raw)
> +{
> + struct IO_APIC_route_entry entry;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&ioapic_lock, flags);
> + entry = __ioapic_read_entry(apic, pin, raw);
> + spin_unlock_irqrestore(&ioapic_lock, flags);
> + return entry;
> +}
> +
> +static void
> +__ioapic_write_entry(int apic, int pin, int raw, struct IO_APIC_route_entry
> e)
> +{
> + void (*write)(unsigned int, unsigned int, unsigned int)
> + = raw ? __io_apic_write : io_apic_write;
> + union entry_union eu = {{0, 0}};
> +
> + eu.entry = e;
> + (*write)(apic, 0x11 + 2*pin, eu.w2);
> + (*write)(apic, 0x10 + 2*pin, eu.w1);
> +}
> +
> +static void ioapic_write_entry(int apic, int pin, int raw, struct
> IO_APIC_route_entry e)
> +{
> + unsigned long flags;
> + spin_lock_irqsave(&ioapic_lock, flags);
> + __ioapic_write_entry(apic, pin, raw, e);
> + spin_unlock_irqrestore(&ioapic_lock, flags);
> +}
> +
> /*
> * Saves all the IO-APIC RTE's
> */
> @@ -170,12 +216,8 @@ int save_IO_APIC_setup(struct IO_APIC_ro
> if (!ioapic_entries[apic])
> return -ENOMEM;
>
> - for (pin = 0; pin < nr_ioapic_registers[apic]; pin++) {
> - *(((int *)&ioapic_entries[apic][pin])+0) =
> - __io_apic_read(apic, 0x10+pin*2);
> - *(((int *)&ioapic_entries[apic][pin])+1) =
> - __io_apic_read(apic, 0x11+pin*2);
> - }
> + for (pin = 0; pin < nr_ioapic_registers[apic]; pin++)
> + ioapic_entries[apic][pin] = __ioapic_read_entry(apic, pin, 1);
> }
>
> return 0;
> @@ -197,16 +239,12 @@ void mask_IO_APIC_setup(struct IO_APIC_r
>
> for (pin = 0; pin < nr_ioapic_registers[apic]; pin++) {
> struct IO_APIC_route_entry entry;
> - unsigned long flags;
>
> entry = ioapic_entries[apic][pin];
> if (!entry.mask) {
> entry.mask = 1;
>
> - spin_lock_irqsave(&ioapic_lock, flags);
> - __io_apic_write(apic, 0x11+2*pin, *(((int *)&entry)+1));
> - __io_apic_write(apic, 0x10+2*pin, *(((int *)&entry)+0));
> - spin_unlock_irqrestore(&ioapic_lock, flags);
> + ioapic_write_entry(apic, pin, 1, entry);
> }
> }
> }
> @@ -218,8 +256,6 @@ void mask_IO_APIC_setup(struct IO_APIC_r
> int restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries)
> {
> int apic, pin;
> - unsigned long flags;
> - struct IO_APIC_route_entry entry;
>
> if (!ioapic_entries)
> return -ENOMEM;
> @@ -229,11 +265,7 @@ int restore_IO_APIC_setup(struct IO_APIC
> return -ENOMEM;
>
> for (pin = 0; pin < nr_ioapic_registers[apic]; pin++)
> - entry = ioapic_entries[apic][pin];
> - spin_lock_irqsave(&ioapic_lock, flags);
> - __io_apic_write(apic, 0x11+2*pin, *(((int *)&entry)+1));
> - __io_apic_write(apic, 0x10+2*pin, *(((int *)&entry)+0));
> - spin_unlock_irqrestore(&ioapic_lock, flags);
> + ioapic_write_entry(apic, pin, 1, ioapic_entries[apic][pin]);
... this for's (intended) body was lacking braces. Hence this
definitely is also a 4.1/4.0 back-porting candidate.
Jan
> }
>
> return 0;
> @@ -338,18 +370,10 @@ static void eoi_IO_APIC_irq(unsigned int
> #define clear_IO_APIC_pin_raw(a,p) __clear_IO_APIC_pin(a,p,1)
> static void __clear_IO_APIC_pin(unsigned int apic, unsigned int pin, int
> raw)
> {
> - unsigned int (*read)(unsigned int, unsigned int)
> - = raw ? __io_apic_read : io_apic_read;
> - void (*write)(unsigned int, unsigned int, unsigned int)
> - = raw ? __io_apic_write : io_apic_write;
> struct IO_APIC_route_entry entry;
> - unsigned long flags;
> -
> +
> /* Check delivery_mode to be sure we're not clearing an SMI pin */
> - spin_lock_irqsave(&ioapic_lock, flags);
> - *(((int*)&entry) + 0) = (*read)(apic, 0x10 + 2 * pin);
> - *(((int*)&entry) + 1) = (*read)(apic, 0x11 + 2 * pin);
> - spin_unlock_irqrestore(&ioapic_lock, flags);
> + entry = ioapic_read_entry(apic, pin, raw);
> if (entry.delivery_mode == dest_SMI)
> return;
>
> @@ -358,10 +382,7 @@ static void __clear_IO_APIC_pin(unsigned
> */
> memset(&entry, 0, sizeof(entry));
> entry.mask = 1;
> - spin_lock_irqsave(&ioapic_lock, flags);
> - (*write)(apic, 0x10 + 2 * pin, *(((int *)&entry) + 0));
> - (*write)(apic, 0x11 + 2 * pin, *(((int *)&entry) + 1));
> - spin_unlock_irqrestore(&ioapic_lock, flags);
> + ioapic_write_entry(apic, pin, raw, entry);
> }
>
> static void clear_IO_APIC (void)
> @@ -990,11 +1011,10 @@ static void __init setup_IO_APIC_irqs(vo
> SET_DEST(entry.dest.dest32, entry.dest.logical.logical_dest,
> cpu_mask_to_apicid(&cfg->cpu_mask));
> spin_lock_irqsave(&ioapic_lock, flags);
> - io_apic_write(apic, 0x11+2*pin, *(((int *)&entry)+1));
> - io_apic_write(apic, 0x10+2*pin, *(((int *)&entry)+0));
> + __ioapic_write_entry(apic, pin, 0, entry);
> set_native_irq_info(irq, TARGET_CPUS);
> spin_unlock_irqrestore(&ioapic_lock, flags);
> - }
> + }
> }
>
> if (!first_notcon)
> @@ -1007,7 +1027,6 @@ static void __init setup_IO_APIC_irqs(vo
> static void __init setup_ExtINT_IRQ0_pin(unsigned int apic, unsigned int
> pin, int vector)
> {
> struct IO_APIC_route_entry entry;
> - unsigned long flags;
>
> memset(&entry,0,sizeof(entry));
>
> @@ -1038,10 +1057,7 @@ static void __init setup_ExtINT_IRQ0_pin
> /*
> * Add it to the IO-APIC irq-routing table:
> */
> - spin_lock_irqsave(&ioapic_lock, flags);
> - io_apic_write(apic, 0x11+2*pin, *(((int *)&entry)+1));
> - io_apic_write(apic, 0x10+2*pin, *(((int *)&entry)+0));
> - spin_unlock_irqrestore(&ioapic_lock, flags);
> + ioapic_write_entry(apic, pin, 0, entry);
>
> enable_8259A_irq(0);
> }
> @@ -1148,10 +1164,7 @@ static void /*__init*/ __print_IO_APIC(v
> for (i = 0; i <= reg_01.bits.entries; i++) {
> struct IO_APIC_route_entry entry;
>
> - spin_lock_irqsave(&ioapic_lock, flags);
> - *(((int *)&entry)+0) = io_apic_read(apic, 0x10+i*2);
> - *(((int *)&entry)+1) = io_apic_read(apic, 0x11+i*2);
> - spin_unlock_irqrestore(&ioapic_lock, flags);
> + entry = ioapic_read_entry(apic, i, 0);
>
> printk(KERN_DEBUG " %02x %03X %02X ",
> i,
> @@ -1212,7 +1225,6 @@ static void __init enable_IO_APIC(void)
> {
> int i8259_apic, i8259_pin;
> int i, apic;
> - unsigned long flags;
>
> /* Initialise dynamic irq_2_pin free list. */
> irq_2_pin = xmalloc_array(struct irq_pin_list, PIN_MAP_SIZE);
> @@ -1227,12 +1239,7 @@ static void __init enable_IO_APIC(void)
> int pin;
> /* See if any of the pins is in ExtINT mode */
> for (pin = 0; pin < nr_ioapic_registers[apic]; pin++) {
> - struct IO_APIC_route_entry entry;
> - spin_lock_irqsave(&ioapic_lock, flags);
> - *(((int *)&entry) + 0) = io_apic_read(apic, 0x10 + 2 * pin);
> - *(((int *)&entry) + 1) = io_apic_read(apic, 0x11 + 2 * pin);
> - spin_unlock_irqrestore(&ioapic_lock, flags);
> -
> + struct IO_APIC_route_entry entry = ioapic_read_entry(apic, pin,
> 0);
>
> /* If the interrupt line is enabled and in ExtInt mode
> * I have found the pin where the i8259 is connected.
> @@ -1288,7 +1295,6 @@ void disable_IO_APIC(void)
> */
> if (ioapic_i8259.pin != -1) {
> struct IO_APIC_route_entry entry;
> - unsigned long flags;
>
> memset(&entry, 0, sizeof(entry));
> entry.mask = 0; /* Enabled */
> @@ -1305,12 +1311,7 @@ void disable_IO_APIC(void)
> /*
> * Add it to the IO-APIC irq-routing table:
> */
> - spin_lock_irqsave(&ioapic_lock, flags);
> - io_apic_write(ioapic_i8259.apic, 0x11+2*ioapic_i8259.pin,
> - *(((int *)&entry)+1));
> - io_apic_write(ioapic_i8259.apic, 0x10+2*ioapic_i8259.pin,
> - *(((int *)&entry)+0));
> - spin_unlock_irqrestore(&ioapic_lock, flags);
> + ioapic_write_entry(ioapic_i8259.apic, ioapic_i8259.pin, 0, entry);
> }
> disconnect_bsp_APIC(ioapic_i8259.pin != -1);
> }
> @@ -1838,17 +1839,13 @@ static void __init unlock_ExtINT_logic(v
> int apic, pin, i;
> struct IO_APIC_route_entry entry0, entry1;
> unsigned char save_control, save_freq_select;
> - unsigned long flags;
>
> pin = find_isa_irq_pin(8, mp_INT);
> apic = find_isa_irq_apic(8, mp_INT);
> if (pin == -1)
> return;
>
> - spin_lock_irqsave(&ioapic_lock, flags);
> - *(((int *)&entry0) + 1) = io_apic_read(apic, 0x11 + 2 * pin);
> - *(((int *)&entry0) + 0) = io_apic_read(apic, 0x10 + 2 * pin);
> - spin_unlock_irqrestore(&ioapic_lock, flags);
> + entry0 = ioapic_read_entry(apic, pin, 0);
> clear_IO_APIC_pin(apic, pin);
>
> memset(&entry1, 0, sizeof(entry1));
> @@ -1862,10 +1859,7 @@ static void __init unlock_ExtINT_logic(v
> entry1.trigger = 0;
> entry1.vector = 0;
>
> - spin_lock_irqsave(&ioapic_lock, flags);
> - io_apic_write(apic, 0x11 + 2 * pin, *(((int *)&entry1) + 1));
> - io_apic_write(apic, 0x10 + 2 * pin, *(((int *)&entry1) + 0));
> - spin_unlock_irqrestore(&ioapic_lock, flags);
> + ioapic_write_entry(apic, pin, 0, entry1);
>
> save_control = CMOS_READ(RTC_CONTROL);
> save_freq_select = CMOS_READ(RTC_FREQ_SELECT);
> @@ -1884,10 +1878,7 @@ static void __init unlock_ExtINT_logic(v
> CMOS_WRITE(save_freq_select, RTC_FREQ_SELECT);
> clear_IO_APIC_pin(apic, pin);
>
> - spin_lock_irqsave(&ioapic_lock, flags);
> - io_apic_write(apic, 0x11 + 2 * pin, *(((int *)&entry0) + 1));
> - io_apic_write(apic, 0x10 + 2 * pin, *(((int *)&entry0) + 0));
> - spin_unlock_irqrestore(&ioapic_lock, flags);
> + ioapic_write_entry(apic, pin, 0, entry0);
> }
>
> /*
> @@ -2262,8 +2253,7 @@ int io_apic_set_pci_routing (int ioapic,
> disable_8259A_irq(irq);
>
> spin_lock_irqsave(&ioapic_lock, flags);
> - io_apic_write(ioapic, 0x11+2*pin, *(((int *)&entry)+1));
> - io_apic_write(ioapic, 0x10+2*pin, *(((int *)&entry)+0));
> + __ioapic_write_entry(ioapic, pin, 0, entry);
> set_native_irq_info(irq, TARGET_CPUS);
> spin_unlock(&ioapic_lock);
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home