All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Kiszka <jan.kiszka@siemens.com>
To: Blue Swirl <blauwirbel@gmail.com>
Cc: "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	Anthony Liguori <aliguori@us.ibm.com>,
	Gleb Natapov <gleb@redhat.com>,
	Marcelo Tosatti <mtosatti@redhat.com>,
	Alexander Graf <agraf@suse.de>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	Avi Kivity <avi@redhat.com>
Subject: Re: [Qemu-devel] [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation
Date: Thu, 03 Feb 2011 20:25:12 +0100	[thread overview]
Message-ID: <4D4B0118.3080904@siemens.com> (raw)
In-Reply-To: <AANLkTikJfU_auYyWtcSBTg59MKX3di13M9JnNPjEFmP4@mail.gmail.com>

On 2011-02-03 20:11, Blue Swirl wrote:
> On Thu, Feb 3, 2011 at 7:06 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2011-02-03 20:01, Blue Swirl wrote:
>>> On Thu, Feb 3, 2011 at 6:01 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>> On 2011-02-03 18:54, Blue Swirl wrote:
>>>>> On Thu, Feb 3, 2011 at 5:43 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>> On 2011-02-03 18:36, Blue Swirl wrote:
>>>>>>> On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>>> On 2011-02-03 18:03, Blue Swirl wrote:
>>>>>>>>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>>>>> The registers of real IOAPICs can be relocated during runtime (via
>>>>>>>>>> chipset registers). We don't support this yet, but qemu-kvm carries the
>>>>>>>>>> current base address in its version 2 vmstate.
>>>>>>>>>>
>>>>>>>>>> To align both implementations for migratability, add the proper
>>>>>>>>>> infrastructure to accept initial as well as updated base addresses and
>>>>>>>>>> include the current address in the vmstate. This is done in a way that
>>>>>>>>>> will also allow multiple IOAPICs in the future.
>>>>>>>>>
>>>>>>>>> Nack, the addresses should be device properties.
>>>>>>>>
>>>>>>>> Hmm.... we could make default_base_address a property. Will change that.
>>>>>>>> But current_base_address is just the same as apicbase and can't be a
>>>>>>>> property.
>>>>>>>
>>>>>>> Oh, right. What will current_base_address used for? Why can't board
>>>>>>> just unmap IOAPIC from current address and remap it at the new
>>>>>>> address? Then the device would not need to know its base address.
>>>>>>
>>>>>> The board could do this. The question is where we put this service, in
>>>>>> the context if the IOAPIC as ioapic_set_base_address (compare to
>>>>>> cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into
>>>>>> each and every board code. In the latter case, the boards would also be
>>>>>> responsible for saving/restoring the address.
>>>>>
>>>>> How is the device relocated? Where are the chipset registers you mention?
>>>>
>>>> Intel's PIIX chipsets contain a register called APICBASE (but it means
>>>> the IOAPIC), and that defines the location. The analogy in the APIC
>>>> world is the MSR_IA32_APICBASE which we maintain via the APIC state.
>>>
>>> In ICH10 the register is called OIC—Other Interrupt Control Register
>>> and the interesting bits APIC Range Select (ASEL).
>>>
>>> So actually PIIX should manage IOAPIC mapping, not board level.
>>
>> The point is we need ioapic_set_base_address logic in multiple places
>> (once chipsets start to implement it). Better push it to a central place
>> from the beginning. Also the bit keeping. There is no difference to
>> apicbase.
> 
> In that case, the function should be made inline version in ioapic.h.

That still replicates the bit keeping.

I don't see the benefit of moving it over, even less when we want to
consolidate with a vmstate layout that is already in use.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

WARNING: multiple messages have this Message-ID (diff)
From: Jan Kiszka <jan.kiszka@siemens.com>
To: Blue Swirl <blauwirbel@gmail.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>,
	Gleb Natapov <gleb@redhat.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	Marcelo Tosatti <mtosatti@redhat.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	Alexander Graf <agraf@suse.de>, Avi Kivity <avi@redhat.com>
Subject: Re: [Qemu-devel] [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation
Date: Thu, 03 Feb 2011 20:25:12 +0100	[thread overview]
Message-ID: <4D4B0118.3080904@siemens.com> (raw)
In-Reply-To: <AANLkTikJfU_auYyWtcSBTg59MKX3di13M9JnNPjEFmP4@mail.gmail.com>

On 2011-02-03 20:11, Blue Swirl wrote:
> On Thu, Feb 3, 2011 at 7:06 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2011-02-03 20:01, Blue Swirl wrote:
>>> On Thu, Feb 3, 2011 at 6:01 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>> On 2011-02-03 18:54, Blue Swirl wrote:
>>>>> On Thu, Feb 3, 2011 at 5:43 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>> On 2011-02-03 18:36, Blue Swirl wrote:
>>>>>>> On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>>> On 2011-02-03 18:03, Blue Swirl wrote:
>>>>>>>>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>>>>> The registers of real IOAPICs can be relocated during runtime (via
>>>>>>>>>> chipset registers). We don't support this yet, but qemu-kvm carries the
>>>>>>>>>> current base address in its version 2 vmstate.
>>>>>>>>>>
>>>>>>>>>> To align both implementations for migratability, add the proper
>>>>>>>>>> infrastructure to accept initial as well as updated base addresses and
>>>>>>>>>> include the current address in the vmstate. This is done in a way that
>>>>>>>>>> will also allow multiple IOAPICs in the future.
>>>>>>>>>
>>>>>>>>> Nack, the addresses should be device properties.
>>>>>>>>
>>>>>>>> Hmm.... we could make default_base_address a property. Will change that.
>>>>>>>> But current_base_address is just the same as apicbase and can't be a
>>>>>>>> property.
>>>>>>>
>>>>>>> Oh, right. What will current_base_address used for? Why can't board
>>>>>>> just unmap IOAPIC from current address and remap it at the new
>>>>>>> address? Then the device would not need to know its base address.
>>>>>>
>>>>>> The board could do this. The question is where we put this service, in
>>>>>> the context if the IOAPIC as ioapic_set_base_address (compare to
>>>>>> cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into
>>>>>> each and every board code. In the latter case, the boards would also be
>>>>>> responsible for saving/restoring the address.
>>>>>
>>>>> How is the device relocated? Where are the chipset registers you mention?
>>>>
>>>> Intel's PIIX chipsets contain a register called APICBASE (but it means
>>>> the IOAPIC), and that defines the location. The analogy in the APIC
>>>> world is the MSR_IA32_APICBASE which we maintain via the APIC state.
>>>
>>> In ICH10 the register is called OIC—Other Interrupt Control Register
>>> and the interesting bits APIC Range Select (ASEL).
>>>
>>> So actually PIIX should manage IOAPIC mapping, not board level.
>>
>> The point is we need ioapic_set_base_address logic in multiple places
>> (once chipsets start to implement it). Better push it to a central place
>> from the beginning. Also the bit keeping. There is no difference to
>> apicbase.
> 
> In that case, the function should be made inline version in ioapic.h.

That still replicates the bit keeping.

I don't see the benefit of moving it over, even less when we want to
consolidate with a vmstate layout that is already in use.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

  reply	other threads:[~2011-02-03 19:25 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-03 14:55 [0.14?][PATCH 0/4] IOAPIC fixes Jan Kiszka
2011-02-03 14:55 ` [Qemu-devel] " Jan Kiszka
2011-02-03 14:55 ` [0.14?][PATCH 1/4] ioapic: Implement EOI handling for level-triggered IRQs Jan Kiszka
2011-02-03 14:55   ` [Qemu-devel] " Jan Kiszka
2011-02-03 19:59   ` Anthony Liguori
2011-02-03 19:59     ` [Qemu-devel] " Anthony Liguori
2011-02-03 14:55 ` [0.14?][PATCH 2/4] ioapic: Save/restore irr Jan Kiszka
2011-02-03 14:55   ` [Qemu-devel] " Jan Kiszka
2011-02-03 14:55 ` [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation Jan Kiszka
2011-02-03 14:55   ` [Qemu-devel] " Jan Kiszka
2011-02-03 17:03   ` Blue Swirl
2011-02-03 17:03     ` Blue Swirl
2011-02-03 17:18     ` Jan Kiszka
2011-02-03 17:18       ` Jan Kiszka
2011-02-03 17:36       ` Blue Swirl
2011-02-03 17:36         ` Blue Swirl
2011-02-03 17:43         ` Jan Kiszka
2011-02-03 17:43           ` Jan Kiszka
2011-02-03 17:54           ` Blue Swirl
2011-02-03 17:54             ` Blue Swirl
2011-02-03 18:01             ` Jan Kiszka
2011-02-03 18:01               ` Jan Kiszka
2011-02-03 19:01               ` Blue Swirl
2011-02-03 19:01                 ` Blue Swirl
2011-02-03 19:06                 ` Jan Kiszka
2011-02-03 19:06                   ` [Qemu-devel] " Jan Kiszka
2011-02-03 19:11                   ` Blue Swirl
2011-02-03 19:11                     ` Blue Swirl
2011-02-03 19:25                     ` Jan Kiszka [this message]
2011-02-03 19:25                       ` Jan Kiszka
2011-02-03 19:30                       ` Blue Swirl
2011-02-03 19:30                         ` Blue Swirl
2011-02-03 19:42                         ` Jan Kiszka
2011-02-03 19:42                           ` Jan Kiszka
2011-02-03 14:55 ` [0.14?][PATCH 4/4] ioapic: Style & magics cleanup Jan Kiszka
2011-02-03 14:55   ` [Qemu-devel] " Jan Kiszka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4D4B0118.3080904@siemens.com \
    --to=jan.kiszka@siemens.com \
    --cc=agraf@suse.de \
    --cc=aliguori@us.ibm.com \
    --cc=avi@redhat.com \
    --cc=blauwirbel@gmail.com \
    --cc=gleb@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=mtosatti@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.