Re: [RFC -tip] x86,io-apic: Do not map IO-APIC direct registers twice

[Yinghai Lu <yinghai@kernel.org>]

Cyrill Gorcunov wrote:
> On Fri, Nov 13, 2009 at 10:56:35AM -0800, Yinghai Lu wrote:
>> Cyrill Gorcunov wrote:
>>> On Thu, Nov 12, 2009 at 03:22:55PM -0800, Yinghai Lu wrote:
>>>> Cyrill Gorcunov wrote:
>>>>> Please review, I didn't manage to test (emulate actually since I don't
>>>>>
>>> ...
>>>
>>> First of all -- thanks a lot for review Yinghai!
>>>
>>>>> Also insert_resourse will not fail anymore on 1K aligned io-apics.
>>>> looks that we don't need that ...
>>>> not io_apic_base already have that + &. left problems are
>>>> 1. display.
>>>> 2. insert resource problem.
>>>>
>>>> YH
>>>>
>>>> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
>>>> index 90e8bc5..6a9379b 100644
>>>> --- a/arch/x86/kernel/apic/io_apic.c
>>>> +++ b/arch/x86/kernel/apic/io_apic.c
>>>> @@ -4140,11 +4140,13 @@ fake_ioapic_page:
>>>>  		set_fixmap_nocache(idx, ioapic_phys);
>>>>  		apic_printk(APIC_VERBOSE,
>>>>  			    "mapped IOAPIC to %08lx (%08lx)\n",
>>>> -			    __fix_to_virt(idx), ioapic_phys);
>>>> +			    __fix_to_virt(idx) + (ioapic_phys & ~PAGE_MASK),
>>> Don't understand, why? What is wrong with physical address,
>>> could you elaborate please?
>> ioapic_phys could be 1k aligned, but __fix_to_virt(idx) will always return 4k aligned.
> 
> yeah, I just misread your patch, I thought you've changed second argument,
> sorry. OK, I see what you mean, good catch, thanks!
> 
>>>> +			    ioapic_phys);
>>>>  		idx++;
>>>>  
>>>> +		/* spec says size is 1024 */
>>> Hmm, MP says nothing about size of IO-APIC direct registers
>>> cound and as a result -- the size of MMIO. It will (and
>>> is) differ between IO-APIC versions. An example -- IO-APIC EOI register
>>> which 82489DX just dont have at all. At moment (ICH-10) the lenght is 68
>>> bytes so you may note the comment in the former patch that we "hope" such
>>> a size will be enough for quite a long time to cover all direct register
>>> space an IO-APIC provides (though to be precise from this 68 bytes only
>>> index,data,eoi registers specified).
>>>
>>>>  		ioapic_res->start = ioapic_phys;
>>>> -		ioapic_res->end = ioapic_phys + PAGE_SIZE-1;
>>>> +		ioapic_res->end = ioapic_phys + (1<<10) - 1;
>>>>  		ioapic_res++;
>>>>  	}
>>>>  }
>>>>
>>> I think I've compicated the patch/idea too much indeed :)
>>> Since we have fixmap for all io_apics build time reserved
>>> even if some io-apic is 1K aligned we still may use new
>>> fixmap index. So only issue remains -- resource allocation.
>>> Here is an updated patch. Please review.
>> 2. print out ...?
> 
> Print out what? Not sure I understand you right. Perhaps you mean
> to check insert_resourse results?

  		apic_printk(APIC_VERBOSE,
 			    "mapped IOAPIC to %08lx (%08lx)\n",
-			    __fix_to_virt(idx), ioapic_phys);
+			    __fix_to_virt(idx) + (ioapic_phys & ~PAGE_MASK),

YH