On 21.03.2013, at 12:09, Peter Maydell wrote:
> On 21 March 2013 11:05, Alexander Graf <ag...@suse.de> wrote:
>>
>> On 21.03.2013, at 12:01, Peter Maydell wrote:
>>
>>> On 21 March 2013 10:59, Alexander Graf <ag...@suse.de> wrote:
>>>> On 21.03.2013, at 11:53, Peter Maydell wrote:
>>>>> Check kvm_arm_register_device() in target-arm/kvm.c. Basically
>>>>> the VGIC device model calls this function to say "tell the kernel
>>>>> where this MemoryRegion is in the system address space, when it
>>>>> eventually gets mapped". The code in kvm.c uses the memory system's
>>>>> Notifier API to get a callback when the region is mapped into
>>>>> an address space, which it uses to track the offset in the
>>>>> address space. Finally, we use a machine init notifier so that
>>>>> just before everything finally starts we can make the KVM ioctls
>>>>> to say "here is where everything lives".
>>>>
>>>> Same thing here. The question is how the kvm-vgic code in QEMU
>>>> finds out where it got mapped to. Scott adds this patch to do
>>>> this, but I'd assume you have some other way :)
>>>
>>> Hmm? The kvm-vgic code in QEMU doesn't need to know where it
>>> lives. We have to tell the kernel so it can map its bits of
>>> registers in at the right place, that's all.
>>
>> The kvm-vgic code in QEMU needs to tell the kernel, no? For
>> that, it needs to know what to tell the kernel.
>
> No. As I explained earlier, all the kvm-vgic code needs to do
> is call kvm_arm_register_device(). That code in kvm.c then takes
> care of telling the kernel. hw/kvm/arm_gic.c itself never knows or
> needs to know where it's mapped. This is the whole point of the
> mechanism involving notifiers.
I fully disagree. Code that talks to the in-kernel device should live in
hw/kvm/<device>.c, not in some random target-XXX/kvm.c file. Of course the
board defines where the device gets mapped to, but the communication with the
in-kernel device bits should really be contained to the device itself.
So this is the function that gets invoked on ARM:
static void kvm_arm_machine_init_done(Notifier *notifier, void *data)
{
KVMDevice *kd, *tkd;
memory_listener_unregister(&devlistener);
QSLIST_FOREACH_SAFE(kd, &kvm_devices_head, entries, tkd) {
if (kd->kda.addr != -1) {
if (kvm_vm_ioctl(kvm_state, KVM_ARM_SET_DEVICE_ADDR,
&kd->kda) < 0) {
fprintf(stderr, "KVM_ARM_SET_DEVICE_ADDRESS failed: %s\n",
strerror(errno));
abort();
}
}
g_free(kd);
}
}
This only goes one level deep, right? So if you ever have to nest the VGIC
inside of another MemoryRegion, this will break, right?
Alex
>
>> This patch adds a function that allows kvm-openpic to fetch its
>> base flat address from the MemoryListener.
>
> This sounds to me like the wrong way to do it -- it's board
> models that decide where devices are mapped and the device
> code itself shouldn't have to know where it has been mapped.
>
> -- PMM
