* Re: [Xen-devel] [PATCH 3/3] xen/x86: ioapic: Simplify ioapic_init()
2020-03-27 19:05 ` [Xen-devel] [PATCH 3/3] xen/x86: ioapic: Simplify ioapic_init() Julien Grall
@ 2020-03-29 14:35 ` Wei Liu
2020-03-29 15:54 ` Julien Grall
2020-03-30 10:43 ` Roger Pau Monné
2020-03-31 11:22 ` Jan Beulich
2 siblings, 1 reply; 16+ messages in thread
From: Wei Liu @ 2020-03-29 14:35 UTC (permalink / raw)
To: Julien Grall
Cc: Wei Liu, Andrew Cooper, Julien Grall, Jan Beulich, xen-devel,
Roger Pau Monné
On Fri, Mar 27, 2020 at 07:05:46PM +0000, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
>
> Since commit 9facd54a45 "x86/ioapic: Add register level checks to detect
> bogus io-apic entries", Xen is able to cope with IO APICs not mapped in
> the fixmap.
>
> Therefore the whole logic to allocate a fake page for some IO APICs is
> unnecessary.
>
> With the logic removed, the code can be simplified a lot as we don't
> need to go through all the IO APIC if SMP has not been detected or a
> bogus zero IO-APIC address has been detected.
>
> To avoid another level of tabulation, the simplification is now moved in
> its own function.
>
> Signed-off-by: Julien Grall <jgrall@amazon.com>
> ---
> xen/arch/x86/io_apic.c | 63 ++++++++++++++++++++----------------------
> 1 file changed, 30 insertions(+), 33 deletions(-)
>
> diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
> index 9a11ee8342..3d52e4daf1 100644
> --- a/xen/arch/x86/io_apic.c
> +++ b/xen/arch/x86/io_apic.c
> @@ -2537,34 +2537,25 @@ static __init bool bad_ioapic_register(unsigned int idx)
> return false;
> }
>
> -void __init init_ioapic(void)
> +static void __init init_ioapic_mappings(void)
> {
> - unsigned long ioapic_phys;
> unsigned int i, idx = FIX_IO_APIC_BASE_0;
> - union IO_APIC_reg_01 reg_01;
>
> - if ( smp_found_config )
> - nr_irqs_gsi = 0;
> for ( i = 0; i < nr_ioapics; i++ )
> {
> - if ( smp_found_config )
> - {
> - ioapic_phys = mp_ioapics[i].mpc_apicaddr;
> - if ( !ioapic_phys )
> - {
> - printk(KERN_ERR "WARNING: bogus zero IO-APIC address "
> - "found in MPTABLE, disabling IO/APIC support!\n");
> - smp_found_config = false;
> - skip_ioapic_setup = true;
> - goto fake_ioapic_page;
> - }
> - }
> - else
> + union IO_APIC_reg_01 reg_01;
> + unsigned long ioapic_phys = mp_ioapics[i].mpc_apicaddr;
> +
> + ioapic_phys = mp_ioapics[i].mpc_apicaddr;
ioapic_phys is set a second time here. See the line before.
Wei.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Xen-devel] [PATCH 3/3] xen/x86: ioapic: Simplify ioapic_init()
2020-03-29 14:35 ` Wei Liu
@ 2020-03-29 15:54 ` Julien Grall
0 siblings, 0 replies; 16+ messages in thread
From: Julien Grall @ 2020-03-29 15:54 UTC (permalink / raw)
To: Wei Liu
Cc: xen-devel, Julien Grall, Roger Pau Monné, Jan Beulich,
Andrew Cooper
Hi Wei,
On 29/03/2020 15:35, Wei Liu wrote:
> On Fri, Mar 27, 2020 at 07:05:46PM +0000, Julien Grall wrote:
>> From: Julien Grall <jgrall@amazon.com>
>>
>> Since commit 9facd54a45 "x86/ioapic: Add register level checks to detect
>> bogus io-apic entries", Xen is able to cope with IO APICs not mapped in
>> the fixmap.
>>
>> Therefore the whole logic to allocate a fake page for some IO APICs is
>> unnecessary.
>>
>> With the logic removed, the code can be simplified a lot as we don't
>> need to go through all the IO APIC if SMP has not been detected or a
>> bogus zero IO-APIC address has been detected.
>>
>> To avoid another level of tabulation, the simplification is now moved in
>> its own function.
>>
>> Signed-off-by: Julien Grall <jgrall@amazon.com>
>> ---
>> xen/arch/x86/io_apic.c | 63 ++++++++++++++++++++----------------------
>> 1 file changed, 30 insertions(+), 33 deletions(-)
>>
>> diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
>> index 9a11ee8342..3d52e4daf1 100644
>> --- a/xen/arch/x86/io_apic.c
>> +++ b/xen/arch/x86/io_apic.c
>> @@ -2537,34 +2537,25 @@ static __init bool bad_ioapic_register(unsigned int idx)
>> return false;
>> }
>>
>> -void __init init_ioapic(void)
>> +static void __init init_ioapic_mappings(void)
>> {
>> - unsigned long ioapic_phys;
>> unsigned int i, idx = FIX_IO_APIC_BASE_0;
>> - union IO_APIC_reg_01 reg_01;
>>
>> - if ( smp_found_config )
>> - nr_irqs_gsi = 0;
>> for ( i = 0; i < nr_ioapics; i++ )
>> {
>> - if ( smp_found_config )
>> - {
>> - ioapic_phys = mp_ioapics[i].mpc_apicaddr;
>> - if ( !ioapic_phys )
>> - {
>> - printk(KERN_ERR "WARNING: bogus zero IO-APIC address "
>> - "found in MPTABLE, disabling IO/APIC support!\n");
>> - smp_found_config = false;
>> - skip_ioapic_setup = true;
>> - goto fake_ioapic_page;
>> - }
>> - }
>> - else
>> + union IO_APIC_reg_01 reg_01;
>> + unsigned long ioapic_phys = mp_ioapics[i].mpc_apicaddr;
>> +
>> + ioapic_phys = mp_ioapics[i].mpc_apicaddr;
>
> ioapic_phys is set a second time here. See the line before.
Good spot! I will drop the line.
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Xen-devel] [PATCH 3/3] xen/x86: ioapic: Simplify ioapic_init()
2020-03-27 19:05 ` [Xen-devel] [PATCH 3/3] xen/x86: ioapic: Simplify ioapic_init() Julien Grall
2020-03-29 14:35 ` Wei Liu
@ 2020-03-30 10:43 ` Roger Pau Monné
2020-03-30 12:56 ` Julien Grall
2020-03-31 11:22 ` Jan Beulich
2 siblings, 1 reply; 16+ messages in thread
From: Roger Pau Monné @ 2020-03-30 10:43 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, Julien Grall, Wei Liu, Jan Beulich, Andrew Cooper
On Fri, Mar 27, 2020 at 07:05:46PM +0000, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
>
> Since commit 9facd54a45 "x86/ioapic: Add register level checks to detect
> bogus io-apic entries", Xen is able to cope with IO APICs not mapped in
> the fixmap.
>
> Therefore the whole logic to allocate a fake page for some IO APICs is
> unnecessary.
>
> With the logic removed, the code can be simplified a lot as we don't
> need to go through all the IO APIC if SMP has not been detected or a
> bogus zero IO-APIC address has been detected.
>
> To avoid another level of tabulation, the simplification is now moved in
> its own function.
>
> Signed-off-by: Julien Grall <jgrall@amazon.com>
> ---
> xen/arch/x86/io_apic.c | 63 ++++++++++++++++++++----------------------
> 1 file changed, 30 insertions(+), 33 deletions(-)
>
> diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
> index 9a11ee8342..3d52e4daf1 100644
> --- a/xen/arch/x86/io_apic.c
> +++ b/xen/arch/x86/io_apic.c
> @@ -2537,34 +2537,25 @@ static __init bool bad_ioapic_register(unsigned int idx)
> return false;
> }
>
> -void __init init_ioapic(void)
> +static void __init init_ioapic_mappings(void)
Likewise my comment in 2/3 I would name this ioapic_init_mappings.
> {
> - unsigned long ioapic_phys;
> unsigned int i, idx = FIX_IO_APIC_BASE_0;
> - union IO_APIC_reg_01 reg_01;
>
> - if ( smp_found_config )
> - nr_irqs_gsi = 0;
> for ( i = 0; i < nr_ioapics; i++ )
> {
> - if ( smp_found_config )
> - {
> - ioapic_phys = mp_ioapics[i].mpc_apicaddr;
> - if ( !ioapic_phys )
> - {
> - printk(KERN_ERR "WARNING: bogus zero IO-APIC address "
> - "found in MPTABLE, disabling IO/APIC support!\n");
> - smp_found_config = false;
> - skip_ioapic_setup = true;
> - goto fake_ioapic_page;
> - }
> - }
> - else
> + union IO_APIC_reg_01 reg_01;
> + unsigned long ioapic_phys = mp_ioapics[i].mpc_apicaddr;
Nit: paddr_t might be better here.
Thanks, Roger.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Xen-devel] [PATCH 3/3] xen/x86: ioapic: Simplify ioapic_init()
2020-03-30 10:43 ` Roger Pau Monné
@ 2020-03-30 12:56 ` Julien Grall
0 siblings, 0 replies; 16+ messages in thread
From: Julien Grall @ 2020-03-30 12:56 UTC (permalink / raw)
To: Roger Pau Monné
Cc: xen-devel, Julien Grall, Wei Liu, Jan Beulich, Andrew Cooper
Hi Roger,
On 30/03/2020 11:43, Roger Pau Monné wrote:
> On Fri, Mar 27, 2020 at 07:05:46PM +0000, Julien Grall wrote:
>> From: Julien Grall <jgrall@amazon.com>
>>
>> Since commit 9facd54a45 "x86/ioapic: Add register level checks to detect
>> bogus io-apic entries", Xen is able to cope with IO APICs not mapped in
>> the fixmap.
>>
>> Therefore the whole logic to allocate a fake page for some IO APICs is
>> unnecessary.
>>
>> With the logic removed, the code can be simplified a lot as we don't
>> need to go through all the IO APIC if SMP has not been detected or a
>> bogus zero IO-APIC address has been detected.
>>
>> To avoid another level of tabulation, the simplification is now moved in
>> its own function.
>>
>> Signed-off-by: Julien Grall <jgrall@amazon.com>
>> ---
>> xen/arch/x86/io_apic.c | 63 ++++++++++++++++++++----------------------
>> 1 file changed, 30 insertions(+), 33 deletions(-)
>>
>> diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
>> index 9a11ee8342..3d52e4daf1 100644
>> --- a/xen/arch/x86/io_apic.c
>> +++ b/xen/arch/x86/io_apic.c
>> @@ -2537,34 +2537,25 @@ static __init bool bad_ioapic_register(unsigned int idx)
>> return false;
>> }
>>
>> -void __init init_ioapic(void)
>> +static void __init init_ioapic_mappings(void)
>
> Likewise my comment in 2/3 I would name this ioapic_init_mappings.
Will do.
>
>> {
>> - unsigned long ioapic_phys;
>> unsigned int i, idx = FIX_IO_APIC_BASE_0;
>> - union IO_APIC_reg_01 reg_01;
>>
>> - if ( smp_found_config )
>> - nr_irqs_gsi = 0;
>> for ( i = 0; i < nr_ioapics; i++ )
>> {
>> - if ( smp_found_config )
>> - {
>> - ioapic_phys = mp_ioapics[i].mpc_apicaddr;
>> - if ( !ioapic_phys )
>> - {
>> - printk(KERN_ERR "WARNING: bogus zero IO-APIC address "
>> - "found in MPTABLE, disabling IO/APIC support!\n");
>> - smp_found_config = false;
>> - skip_ioapic_setup = true;
>> - goto fake_ioapic_page;
>> - }
>> - }
>> - else
>> + union IO_APIC_reg_01 reg_01;
>> + unsigned long ioapic_phys = mp_ioapics[i].mpc_apicaddr;
>
> Nit: paddr_t might be better here.
Sure.
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] xen/x86: ioapic: Simplify ioapic_init()
2020-03-27 19:05 ` [Xen-devel] [PATCH 3/3] xen/x86: ioapic: Simplify ioapic_init() Julien Grall
2020-03-29 14:35 ` Wei Liu
2020-03-30 10:43 ` Roger Pau Monné
@ 2020-03-31 11:22 ` Jan Beulich
2020-03-31 11:51 ` Julien Grall
2 siblings, 1 reply; 16+ messages in thread
From: Jan Beulich @ 2020-03-31 11:22 UTC (permalink / raw)
To: Julien Grall
Cc: xen-devel, Julien Grall, Roger Pau Monné, Wei Liu,
Andrew Cooper
On 27.03.2020 20:05, Julien Grall wrote:
> --- a/xen/arch/x86/io_apic.c
> +++ b/xen/arch/x86/io_apic.c
> @@ -2537,34 +2537,25 @@ static __init bool bad_ioapic_register(unsigned int idx)
> return false;
> }
>
> -void __init init_ioapic(void)
> +static void __init init_ioapic_mappings(void)
> {
> - unsigned long ioapic_phys;
> unsigned int i, idx = FIX_IO_APIC_BASE_0;
> - union IO_APIC_reg_01 reg_01;
>
> - if ( smp_found_config )
> - nr_irqs_gsi = 0;
> for ( i = 0; i < nr_ioapics; i++ )
> {
> - if ( smp_found_config )
> - {
> - ioapic_phys = mp_ioapics[i].mpc_apicaddr;
> - if ( !ioapic_phys )
> - {
> - printk(KERN_ERR "WARNING: bogus zero IO-APIC address "
> - "found in MPTABLE, disabling IO/APIC support!\n");
> - smp_found_config = false;
> - skip_ioapic_setup = true;
> - goto fake_ioapic_page;
> - }
> - }
> - else
> + union IO_APIC_reg_01 reg_01;
> + unsigned long ioapic_phys = mp_ioapics[i].mpc_apicaddr;
> +
> + ioapic_phys = mp_ioapics[i].mpc_apicaddr;
> + if ( !ioapic_phys )
> {
> - fake_ioapic_page:
> - ioapic_phys = __pa(alloc_xenheap_page());
> - clear_page(__va(ioapic_phys));
> + printk(KERN_ERR
> + "WARNING: bogus zero IO-APIC address found in MPTABLE, disabling IO/APIC support!\n");
> + smp_found_config = false;
> + skip_ioapic_setup = true;
> + break;
> }
> +
> set_fixmap_nocache(idx, ioapic_phys);
> apic_printk(APIC_VERBOSE, "mapped IOAPIC to %08Lx (%08lx)\n",
> __fix_to_virt(idx), ioapic_phys);
> @@ -2576,18 +2567,24 @@ void __init init_ioapic(void)
> continue;
> }
>
> - if ( smp_found_config )
> - {
> - /* The number of IO-APIC IRQ registers (== #pins): */
> - reg_01.raw = io_apic_read(i, 1);
> - nr_ioapic_entries[i] = reg_01.bits.entries + 1;
> - nr_irqs_gsi += nr_ioapic_entries[i];
> -
> - if ( rangeset_add_singleton(mmio_ro_ranges,
> - ioapic_phys >> PAGE_SHIFT) )
> - printk(KERN_ERR "Failed to mark IO-APIC page %lx read-only\n",
> - ioapic_phys);
> - }
> + /* The number of IO-APIC IRQ registers (== #pins): */
> + reg_01.raw = io_apic_read(i, 1);
> + nr_ioapic_entries[i] = reg_01.bits.entries + 1;
> + nr_irqs_gsi += nr_ioapic_entries[i];
> +
> + if ( rangeset_add_singleton(mmio_ro_ranges,
> + ioapic_phys >> PAGE_SHIFT) )
> + printk(KERN_ERR "Failed to mark IO-APIC page %lx read-only\n",
> + ioapic_phys);
> + }
> +}
> +
> +void __init init_ioapic(void)
> +{
> + if ( smp_found_config )
> + {
> + nr_irqs_gsi = 0;
This would seem to also belong into the function, e.g. as part of
the loop header:
for ( nr_irqs_gsi = i = 0; i < nr_ioapics; i++ )
Jan
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 3/3] xen/x86: ioapic: Simplify ioapic_init()
2020-03-31 11:22 ` Jan Beulich
@ 2020-03-31 11:51 ` Julien Grall
2020-03-31 12:12 ` Jan Beulich
0 siblings, 1 reply; 16+ messages in thread
From: Julien Grall @ 2020-03-31 11:51 UTC (permalink / raw)
To: Jan Beulich
Cc: xen-devel, Julien Grall, Roger Pau Monné, Wei Liu,
Andrew Cooper
Hi Jan,
On 31/03/2020 12:22, Jan Beulich wrote:
> On 27.03.2020 20:05, Julien Grall wrote:
>> --- a/xen/arch/x86/io_apic.c
>> +++ b/xen/arch/x86/io_apic.c
>> @@ -2537,34 +2537,25 @@ static __init bool bad_ioapic_register(unsigned int idx)
>> return false;
>> }
>>
>> -void __init init_ioapic(void)
>> +static void __init init_ioapic_mappings(void)
>> {
>> - unsigned long ioapic_phys;
>> unsigned int i, idx = FIX_IO_APIC_BASE_0;
>> - union IO_APIC_reg_01 reg_01;
>>
>> - if ( smp_found_config )
>> - nr_irqs_gsi = 0;
>> for ( i = 0; i < nr_ioapics; i++ )
>> {
>> - if ( smp_found_config )
>> - {
>> - ioapic_phys = mp_ioapics[i].mpc_apicaddr;
>> - if ( !ioapic_phys )
>> - {
>> - printk(KERN_ERR "WARNING: bogus zero IO-APIC address "
>> - "found in MPTABLE, disabling IO/APIC support!\n");
>> - smp_found_config = false;
>> - skip_ioapic_setup = true;
>> - goto fake_ioapic_page;
>> - }
>> - }
>> - else
>> + union IO_APIC_reg_01 reg_01;
>> + unsigned long ioapic_phys = mp_ioapics[i].mpc_apicaddr;
>> +
>> + ioapic_phys = mp_ioapics[i].mpc_apicaddr;
>> + if ( !ioapic_phys )
>> {
>> - fake_ioapic_page:
>> - ioapic_phys = __pa(alloc_xenheap_page());
>> - clear_page(__va(ioapic_phys));
>> + printk(KERN_ERR
>> + "WARNING: bogus zero IO-APIC address found in MPTABLE, disabling IO/APIC support!\n");
>> + smp_found_config = false;
>> + skip_ioapic_setup = true;
>> + break;
>> }
>> +
>> set_fixmap_nocache(idx, ioapic_phys);
>> apic_printk(APIC_VERBOSE, "mapped IOAPIC to %08Lx (%08lx)\n",
>> __fix_to_virt(idx), ioapic_phys);
>> @@ -2576,18 +2567,24 @@ void __init init_ioapic(void)
>> continue;
>> }
>>
>> - if ( smp_found_config )
>> - {
>> - /* The number of IO-APIC IRQ registers (== #pins): */
>> - reg_01.raw = io_apic_read(i, 1);
>> - nr_ioapic_entries[i] = reg_01.bits.entries + 1;
>> - nr_irqs_gsi += nr_ioapic_entries[i];
>> -
>> - if ( rangeset_add_singleton(mmio_ro_ranges,
>> - ioapic_phys >> PAGE_SHIFT) )
>> - printk(KERN_ERR "Failed to mark IO-APIC page %lx read-only\n",
>> - ioapic_phys);
>> - }
>> + /* The number of IO-APIC IRQ registers (== #pins): */
>> + reg_01.raw = io_apic_read(i, 1);
>> + nr_ioapic_entries[i] = reg_01.bits.entries + 1;
>> + nr_irqs_gsi += nr_ioapic_entries[i];
>> +
>> + if ( rangeset_add_singleton(mmio_ro_ranges,
>> + ioapic_phys >> PAGE_SHIFT) )
>> + printk(KERN_ERR "Failed to mark IO-APIC page %lx read-only\n",
>> + ioapic_phys);
>> + }
>> +}
>> +
>> +void __init init_ioapic(void)
>> +{
>> + if ( smp_found_config )
>> + {
>> + nr_irqs_gsi = 0;
>
> This would seem to also belong into the function, e.g. as part of
> the loop header:
>
> for ( nr_irqs_gsi = i = 0; i < nr_ioapics; i++ )
While the initial value of the 'i' and 'nr_irqs_gsi' is the same, the
variables have very different meaning and may confuse the reader.
So I would rather not follow your suggestion here. Although, I can move
the zeroing right before the for loop.
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 3/3] xen/x86: ioapic: Simplify ioapic_init()
2020-03-31 11:51 ` Julien Grall
@ 2020-03-31 12:12 ` Jan Beulich
0 siblings, 0 replies; 16+ messages in thread
From: Jan Beulich @ 2020-03-31 12:12 UTC (permalink / raw)
To: Julien Grall
Cc: xen-devel, Julien Grall, Roger Pau Monné, Wei Liu,
Andrew Cooper
On 31.03.2020 13:51, Julien Grall wrote:
> Hi Jan,
>
> On 31/03/2020 12:22, Jan Beulich wrote:
>> On 27.03.2020 20:05, Julien Grall wrote:
>>> --- a/xen/arch/x86/io_apic.c
>>> +++ b/xen/arch/x86/io_apic.c
>>> @@ -2537,34 +2537,25 @@ static __init bool bad_ioapic_register(unsigned int idx)
>>> return false;
>>> }
>>> -void __init init_ioapic(void)
>>> +static void __init init_ioapic_mappings(void)
>>> {
>>> - unsigned long ioapic_phys;
>>> unsigned int i, idx = FIX_IO_APIC_BASE_0;
>>> - union IO_APIC_reg_01 reg_01;
>>> - if ( smp_found_config )
>>> - nr_irqs_gsi = 0;
>>> for ( i = 0; i < nr_ioapics; i++ )
>>> {
>>> - if ( smp_found_config )
>>> - {
>>> - ioapic_phys = mp_ioapics[i].mpc_apicaddr;
>>> - if ( !ioapic_phys )
>>> - {
>>> - printk(KERN_ERR "WARNING: bogus zero IO-APIC address "
>>> - "found in MPTABLE, disabling IO/APIC support!\n");
>>> - smp_found_config = false;
>>> - skip_ioapic_setup = true;
>>> - goto fake_ioapic_page;
>>> - }
>>> - }
>>> - else
>>> + union IO_APIC_reg_01 reg_01;
>>> + unsigned long ioapic_phys = mp_ioapics[i].mpc_apicaddr;
>>> +
>>> + ioapic_phys = mp_ioapics[i].mpc_apicaddr;
>>> + if ( !ioapic_phys )
>>> {
>>> - fake_ioapic_page:
>>> - ioapic_phys = __pa(alloc_xenheap_page());
>>> - clear_page(__va(ioapic_phys));
>>> + printk(KERN_ERR
>>> + "WARNING: bogus zero IO-APIC address found in MPTABLE, disabling IO/APIC support!\n");
>>> + smp_found_config = false;
>>> + skip_ioapic_setup = true;
>>> + break;
>>> }
>>> +
>>> set_fixmap_nocache(idx, ioapic_phys);
>>> apic_printk(APIC_VERBOSE, "mapped IOAPIC to %08Lx (%08lx)\n",
>>> __fix_to_virt(idx), ioapic_phys);
>>> @@ -2576,18 +2567,24 @@ void __init init_ioapic(void)
>>> continue;
>>> }
>>> - if ( smp_found_config )
>>> - {
>>> - /* The number of IO-APIC IRQ registers (== #pins): */
>>> - reg_01.raw = io_apic_read(i, 1);
>>> - nr_ioapic_entries[i] = reg_01.bits.entries + 1;
>>> - nr_irqs_gsi += nr_ioapic_entries[i];
>>> -
>>> - if ( rangeset_add_singleton(mmio_ro_ranges,
>>> - ioapic_phys >> PAGE_SHIFT) )
>>> - printk(KERN_ERR "Failed to mark IO-APIC page %lx read-only\n",
>>> - ioapic_phys);
>>> - }
>>> + /* The number of IO-APIC IRQ registers (== #pins): */
>>> + reg_01.raw = io_apic_read(i, 1);
>>> + nr_ioapic_entries[i] = reg_01.bits.entries + 1;
>>> + nr_irqs_gsi += nr_ioapic_entries[i];
>>> +
>>> + if ( rangeset_add_singleton(mmio_ro_ranges,
>>> + ioapic_phys >> PAGE_SHIFT) )
>>> + printk(KERN_ERR "Failed to mark IO-APIC page %lx read-only\n",
>>> + ioapic_phys);
>>> + }
>>> +}
>>> +
>>> +void __init init_ioapic(void)
>>> +{
>>> + if ( smp_found_config )
>>> + {
>>> + nr_irqs_gsi = 0;
>>
>> This would seem to also belong into the function, e.g. as part of
>> the loop header:
>>
>> for ( nr_irqs_gsi = i = 0; i < nr_ioapics; i++ )
>
> While the initial value of the 'i' and 'nr_irqs_gsi' is the same,
> the variables have very different meaning and may confuse the reader.
I expected reservations like this, hence the "e.g."; i.e. ...
> So I would rather not follow your suggestion here. Although, I can
> move the zeroing right before the for loop.
... I'm fine with this as well.
Jan
^ permalink raw reply [flat|nested] 16+ messages in thread